[PATCH v3] swupdate_async_start: fix hang with update containing many small steps

[Dominique Martinet]

Running swupdate with a SWU (streamed through swupdate_async_start such
as with `swupdate -i` or `swupdate-client`) can hang if the update
contains many small steps followed by a large step:
- swupdate_async_thread() is stuck trying to write the SWU content to
the server
- but the server sent many progress events and filled progressfd
backlog, and closes the server side of in send_progress_msg() after
retries failed (client is still blocked)
- when server continued installing more items the client is unblocked
and reads up on progress notifications, notices the socket is closed
(POLLHUP) and fails
- the server fails the update as it is not complete

This fails with logs similar to this:
```
progress_ipc_receive_nb failed (-1)
Cannot consume progress events. Fail.
[ERROR] : SWUPDATE failed [0] ERROR install_from_file.c : endupdate : 55 : SWUpdate *failed* !
[TRACE] : SWUPDATE running : [unlink_sockets] : unlink socket /tmp/swupdateprog
[TRACE] : SWUPDATE running : [unlink_sockets] : unlink socket /tmp/sockinstctrl
[ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : hash_compare : 521 : HASH mismatch : 4395313e6f79b6554521e1933304e95d0bea836d2bc72e9b3828fd62c5125baf <--> 3eb039c6666dfd614a73b930c98c38ab77ba362ce3b9d745c7684d0d10335726
```

This patch makes the sending socket nonblocking, allows partial sends of
SWU content (there is no need for it to be a full write unlike IPC
messages), and polls both progressfd and connfd to handle both sockets
simultaneously.

The poll is somewhat redundant with the other poll on progressfd in
progress_ipc_receive_nb(), but progress_ipc_receive_nb() is part of the
progress_ipc API and cannot be changed, so is left as is.

Fixes: 8b4efc0d23db ("swupdate_async_start: fix early termination blocking in client")
Link: https://groups.google.com/g/swupdate/c/F3XKUdP0KTc/m/m2jk4EdkBgAJ
Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
Congrats on the 2026.05 release!

This v3 isn't meant to rush you, I just had a cleanup sitting there and
figured I'd send it before you try the patch.


v2 -> v3:
- remove constant nfd variable (no behaviour change)

v1 -> v2:
- add SoB
- fix hang when trying to install empty image: if size == 0 then we
shouldn't poll at all, because the server didn't get anything to
unblock us and end the loop.
This somewhat simplifies the logic a bit too.

ipc/network_ipc-if.c | 54 +++++++++++++++++++++++++++++++++++---------
ipc/network_ipc.c | 5 ++++
2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/ipc/network_ipc-if.c b/ipc/network_ipc-if.c
index 7c209ad484fc..08b4dbc32f8a 100644
--- a/ipc/network_ipc-if.c
+++ b/ipc/network_ipc-if.c
@@ -5,13 +5,15 @@
* SPDX-License-Identifier: LGPL-2.1-or-later
*/

+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <poll.h>
+#include <pthread.h>
+#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <errno.h>
-#include <signal.h>
-#include <pthread.h>
-#include <inttypes.h>
#include <unistd.h>
#include "network_ipc.h"
#include "progress_ipc.h"
@@ -155,14 +157,15 @@ static int consume_progress_events(int *progressfd)
static void *swupdate_async_thread(void *data)
{
char *pbuf;
- int size;
+ int size = 0, buf_offset = 0;
+ struct pollfd pfds[2];
sigset_t sigpipe_mask;
sigset_t saved_mask;
struct timespec zerotime = {0, 0};
struct async_lib *rq = (struct async_lib *)data;
int swupdate_result = FAILURE;
int progressfd = -1;
- int ret;
+ int ret, old_flags;
int early_status = -1;

sigemptyset(&sigpipe_mask);
@@ -183,19 +186,48 @@ static void *swupdate_async_thread(void *data)
goto out;
}

+ /* make connfd non-blocking while we're writing image */
+ old_flags = fcntl(rq->connfd, F_GETFL);
+ if (old_flags < 0) {
+ fprintf(stderr, "Could not get connfd flags? %m\n");
+ goto out;
+ }
+ fcntl(rq->connfd, F_SETFL, old_flags | O_NONBLOCK);
+
/* Start writing the image until end */

do {
if (!rq->wr)
break;
+ if (size == buf_offset) {
+ buf_offset = 0;
+ rq->wr(&pbuf, &size);
+ }

- rq->wr(&pbuf, &size);
if (size) {
- if (swupdate_image_write(pbuf, size) != size) {
- perror("swupdate_image_write failed");
- swupdate_result = FAILURE;
+ pfds[0].fd = progressfd;
+ pfds[0].events = POLLIN;
+ pfds[1].fd = rq->connfd;
+ pfds[1].events = POLLOUT;
+ do {
+ ret = poll(pfds, 2, -1);
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret < 0) {
+ fprintf(stderr, "poll failed: %m\n");
goto out;
}
+ if (pfds[1].revents & POLLOUT) {
+ ret = swupdate_image_write(pbuf + buf_offset,
+ size - buf_offset);
+ if (ret < 0 && errno != EAGAIN) {
+ perror("swupdate_image_write failed");
+ swupdate_result = FAILURE;
+ goto out;
+ } else if (ret > 0) {
+ buf_offset += ret;
+ }
+ }
}
/* Consume progress events so that the pipe does not get full
* and block the daemon */
@@ -214,7 +246,7 @@ static void *swupdate_async_thread(void *data)
/* interrupt the transfer */
break;
}
- } while(size > 0);
+ } while (size > 0);

ipc_end(rq->connfd);

diff --git a/ipc/network_ipc.c b/ipc/network_ipc.c
index 96a7701d0c9a..ede80dfcffe7 100644
--- a/ipc/network_ipc.c
+++ b/ipc/network_ipc.c
@@ -320,6 +320,11 @@ int ipc_send_data(int connfd, char *buf, int size)

while (len) {
ret = write(connfd, buf, (size_t)len);
+ if (ret < 0 && errno == EINTR)
+ continue;
+ /* return partially written length if any */
+ if (ret < 0 && errno == EAGAIN && len != size)
+ return size - len;
if (ret < 0)
return ret;
len -= ret;
--
2.47.3

[Dominique Martinet]

Hi Stefano,

I rebased the other day and noticed this has been left behind.

My understanding is that you pretty much agreed in principle on v2
(that this is fine to make socket non-blocking here as the socket
doesn't leave the API here and it doesn't impact the rest of the API)
but if there still are concerns I'm happy to consider/rework anything,
please say.

> v2 -> v3:
> - remove constant nfd variable (no behaviour change)
>
> v1 -> v2:
> - add SoB
> - fix hang when trying to install empty image: if size == 0 then we
> shouldn't poll at all, because the server didn't get anything to
> unblock us and end the loop.
> This somewhat simplifies the logic a bit too.

--
Dominique