| Message ID | 20240321054146.2303245-1-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] network_thread: block SIGPIPE in network thread | expand |
On 21.03.24 06:41, Dominique Martinet wrote: > From: Shiita ISHIGAKI <shiita.ishigaki@atmark-techno.com> > > SIGPIPE was blocked in the subprocess thread since commit a781be1d0b7a > ("network_thread: Don't SIGPIPE-die on IPC client crash"), but the main > thread sometimes sends reply through write() directly (for e.g. STATUS) > > In this case, swupdate crashes if the client is gone, which is not > something we want. > > Since we block SIGPIPE before creating the subprocess thread we no > longer need to block it there. > > (Add errno in write error message while we are here) > > Signed-off-by: Shiita ISHIGAKI <shiita.ishigaki@atmark-techno.com> > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > We had a weird swupdate failure in "hawkbit mode" that turned out to be > two different problems, this appears to fix the issue and I checked > SIGPIPE is still blocked in the subprocess_thread but please > double-check. > > core/network_thread.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/core/network_thread.c b/core/network_thread.c > index ca23908d0cda..d7b713fb2154 100644 > --- a/core/network_thread.c > +++ b/core/network_thread.c > @@ -337,11 +337,6 @@ static void *subprocess_thread (void *data) > (void)data; > thread_ready(); > > - sigset_t sigpipe_mask; > - sigemptyset(&sigpipe_mask); > - sigaddset(&sigpipe_mask, SIGPIPE); > - pthread_sigmask(SIG_BLOCK, &sigpipe_mask, NULL); > - > pthread_mutex_lock(&subprocess_msg_lock); > > while(1) { > @@ -393,6 +388,11 @@ void *network_thread (void *data) > SIMPLEQ_INIT(&subprocess_messages); > register_notifier(network_notifier); > > + sigset_t sigpipe_mask; > + sigemptyset(&sigpipe_mask); > + sigaddset(&sigpipe_mask, SIGPIPE); > + pthread_sigmask(SIG_BLOCK, &sigpipe_mask, NULL); > + I think it is ok and it is safe to block SIGPIPE directly in the main thread. Acked-by: Stefano Babic <stefano.babic@swupdate.org> Best regards, Stefano Babic > subprocess_ipc_handler_thread_id = start_thread(subprocess_thread, NULL); > > /* Initialize and bind to UDS */ > @@ -644,7 +644,7 @@ void *network_thread (void *data) > if (msg.type == ACK || msg.type == NACK) { > ret = write(ctrlconnfd, &msg, sizeof(msg)); > if (ret < 0) > - ERROR("Error write on socket ctrl"); > + ERROR("Error write on socket ctrl: %s", strerror(errno)); > > if (should_close_socket == true) > close(ctrlconnfd);
diff --git a/core/network_thread.c b/core/network_thread.c index ca23908d0cda..d7b713fb2154 100644 --- a/core/network_thread.c +++ b/core/network_thread.c @@ -337,11 +337,6 @@ static void *subprocess_thread (void *data) (void)data; thread_ready(); - sigset_t sigpipe_mask; - sigemptyset(&sigpipe_mask); - sigaddset(&sigpipe_mask, SIGPIPE); - pthread_sigmask(SIG_BLOCK, &sigpipe_mask, NULL); - pthread_mutex_lock(&subprocess_msg_lock); while(1) { @@ -393,6 +388,11 @@ void *network_thread (void *data) SIMPLEQ_INIT(&subprocess_messages); register_notifier(network_notifier); + sigset_t sigpipe_mask; + sigemptyset(&sigpipe_mask); + sigaddset(&sigpipe_mask, SIGPIPE); + pthread_sigmask(SIG_BLOCK, &sigpipe_mask, NULL); + subprocess_ipc_handler_thread_id = start_thread(subprocess_thread, NULL); /* Initialize and bind to UDS */ @@ -644,7 +644,7 @@ void *network_thread (void *data) if (msg.type == ACK || msg.type == NACK) { ret = write(ctrlconnfd, &msg, sizeof(msg)); if (ret < 0) - ERROR("Error write on socket ctrl"); + ERROR("Error write on socket ctrl: %s", strerror(errno)); if (should_close_socket == true) close(ctrlconnfd);