| Message ID | 20220506073727.3510746-1-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] network_ipc async_thread: fix hang on failed update | expand |
On 06.05.22 09:37, Dominique Martinet wrote: > if a update is large and fails, it's likely write_image would fail. > Old code would just exit there, quitting swupdate but a recent cleanup > made this just exit async_thread without calling the end function, so > the caller would be left hanging waiting for an update that will never > finish. > > This can easily be reproduced with a large script that exits immediately > with a failure code with swupdate -i or swupdate_client. > > Fixes: 626d83f8819d ("IPC: do not call exit") > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > v1->v2: > - taht -> that typo in commit message > - fix leftover msg.data.status references > > Sorry for the mixup, I don't remember any other conflict when I rebased > these so don't think there'll be any other problem like this but I'll > double-check now and send another mail if required. > > ipc/network_ipc-if.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/ipc/network_ipc-if.c b/ipc/network_ipc-if.c > index c8a6cd02c72f..0f28dadcc137 100644 > --- a/ipc/network_ipc-if.c > +++ b/ipc/network_ipc-if.c > @@ -11,6 +11,7 @@ > #include <errno.h> > #include <signal.h> > #include <pthread.h> > +#include <inttypes.h> > #include "network_ipc.h" > > static pthread_t async_thread_id; > @@ -37,14 +38,15 @@ static void *swupdate_async_thread(void *data) > sigset_t saved_mask; > struct timespec zerotime = {0, 0}; > struct async_lib *rq = (struct async_lib *)data; > - int swupdate_result; > + int swupdate_result = FAILURE; > > sigemptyset(&sigpipe_mask); > sigaddset(&sigpipe_mask, SIGPIPE); > > if (pthread_sigmask(SIG_BLOCK, &sigpipe_mask, &saved_mask) == -1) { > perror("pthread_sigmask"); > - pthread_exit((void *)-1); > + swupdate_result = FAILURE; > + goto out; > } > /* Start writing the image until end */ > > @@ -56,7 +58,8 @@ static void *swupdate_async_thread(void *data) > if (size) { > if (swupdate_image_write(pbuf, size) != size) { > perror("swupdate_image_write failed"); > - pthread_exit((void *)-1); > + swupdate_result = FAILURE; > + goto out; > } > } > } while(size > 0); > @@ -69,20 +72,22 @@ static void *swupdate_async_thread(void *data) > > swupdate_result = ipc_wait_for_complete(rq->get); > > - handle = 0; > - > if (sigtimedwait(&sigpipe_mask, 0, &zerotime) == -1) { > // currently ignored > } > > if (pthread_sigmask(SIG_SETMASK, &saved_mask, 0) == -1) { > - perror("pthread_sigmask"); > + perror("pthread_sigmask"); > + swupdate_result = FAILURE; > + goto out; > } > > +out: > + handle = 0; > if (rq->end) > rq->end((RECOVERY_STATUS)swupdate_result); > > - pthread_exit(NULL); > + pthread_exit((void*)(intptr_t)(swupdate_result == SUCCESS)); > } > > /* Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/ipc/network_ipc-if.c b/ipc/network_ipc-if.c index c8a6cd02c72f..0f28dadcc137 100644 --- a/ipc/network_ipc-if.c +++ b/ipc/network_ipc-if.c @@ -11,6 +11,7 @@ #include <errno.h> #include <signal.h> #include <pthread.h> +#include <inttypes.h> #include "network_ipc.h" static pthread_t async_thread_id; @@ -37,14 +38,15 @@ static void *swupdate_async_thread(void *data) sigset_t saved_mask; struct timespec zerotime = {0, 0}; struct async_lib *rq = (struct async_lib *)data; - int swupdate_result; + int swupdate_result = FAILURE; sigemptyset(&sigpipe_mask); sigaddset(&sigpipe_mask, SIGPIPE); if (pthread_sigmask(SIG_BLOCK, &sigpipe_mask, &saved_mask) == -1) { perror("pthread_sigmask"); - pthread_exit((void *)-1); + swupdate_result = FAILURE; + goto out; } /* Start writing the image until end */ @@ -56,7 +58,8 @@ static void *swupdate_async_thread(void *data) if (size) { if (swupdate_image_write(pbuf, size) != size) { perror("swupdate_image_write failed"); - pthread_exit((void *)-1); + swupdate_result = FAILURE; + goto out; } } } while(size > 0); @@ -69,20 +72,22 @@ static void *swupdate_async_thread(void *data) swupdate_result = ipc_wait_for_complete(rq->get); - handle = 0; - if (sigtimedwait(&sigpipe_mask, 0, &zerotime) == -1) { // currently ignored } if (pthread_sigmask(SIG_SETMASK, &saved_mask, 0) == -1) { - perror("pthread_sigmask"); + perror("pthread_sigmask"); + swupdate_result = FAILURE; + goto out; } +out: + handle = 0; if (rq->end) rq->end((RECOVERY_STATUS)swupdate_result); - pthread_exit(NULL); + pthread_exit((void*)(intptr_t)(swupdate_result == SUCCESS)); } /*
if a update is large and fails, it's likely write_image would fail. Old code would just exit there, quitting swupdate but a recent cleanup made this just exit async_thread without calling the end function, so the caller would be left hanging waiting for an update that will never finish. This can easily be reproduced with a large script that exits immediately with a failure code with swupdate -i or swupdate_client. Fixes: 626d83f8819d ("IPC: do not call exit") Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- v1->v2: - taht -> that typo in commit message - fix leftover msg.data.status references Sorry for the mixup, I don't remember any other conflict when I rebased these so don't think there'll be any other problem like this but I'll double-check now and send another mail if required. ipc/network_ipc-if.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)