| Message ID | 20211124005806.3670879-1-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | network_initializer: move cleanup_files before going IDLE | expand |
On 24.11.21 01:58, Dominique Martinet wrote: > with short-lived swupdate run (swupdate -i) it's possible for the main > thread to exit before cleanup is finished, leaving orphan files behind. > Move the cleanup before the main loop goes idle to make sure cleanup > has had time to finish first > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > This is the follow-up I had promised on Monday: I made it a different > patch as it might potentially affect other update types such as -d as > well, as main process will exit as soon as it gets SIGCHLD and -d will > exit as soon as ipc_wait_for_complete returns. > > (It's just less likely to be a problem as it's still sleep(1)-based, > so the window for the race is much smaller) > > > Feel free to squash with the previous patch (swupdate -i: remove > cleanup_files call in main()) instead if you prefer. > > core/stream_interface.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 31d0de890a2f..1c9853d72f5f 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -653,12 +653,6 @@ void *network_initializer(void *data) > */ > software->parms = parms; > > - pthread_mutex_lock(&stream_mutex); > - inst.status = IDLE; > - pthread_mutex_unlock(&stream_mutex); > - TRACE("Main thread sleep again !"); > - notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); > - > /* release temp files we may have created */ > cleanup_files(software); > > @@ -666,6 +660,12 @@ void *network_initializer(void *data) > swupdate_remove_directory(SCRIPTS_DIR_SUFFIX); > swupdate_remove_directory(DATADST_DIR_SUFFIX); > #endif > + > + pthread_mutex_lock(&stream_mutex); > + inst.status = IDLE; > + pthread_mutex_unlock(&stream_mutex); > + TRACE("Main thread sleep again !"); > + notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); > } > > pthread_exit((void *)0); > Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
diff --git a/core/stream_interface.c b/core/stream_interface.c index 31d0de890a2f..1c9853d72f5f 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -653,12 +653,6 @@ void *network_initializer(void *data) */ software->parms = parms; - pthread_mutex_lock(&stream_mutex); - inst.status = IDLE; - pthread_mutex_unlock(&stream_mutex); - TRACE("Main thread sleep again !"); - notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); - /* release temp files we may have created */ cleanup_files(software); @@ -666,6 +660,12 @@ void *network_initializer(void *data) swupdate_remove_directory(SCRIPTS_DIR_SUFFIX); swupdate_remove_directory(DATADST_DIR_SUFFIX); #endif + + pthread_mutex_lock(&stream_mutex); + inst.status = IDLE; + pthread_mutex_unlock(&stream_mutex); + TRACE("Main thread sleep again !"); + notify(IDLE, RECOVERY_NO_ERROR, INFOLEVEL, "Waiting for requests..."); } pthread_exit((void *)0);
with short-lived swupdate run (swupdate -i) it's possible for the main thread to exit before cleanup is finished, leaving orphan files behind. Move the cleanup before the main loop goes idle to make sure cleanup has had time to finish first Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- This is the follow-up I had promised on Monday: I made it a different patch as it might potentially affect other update types such as -d as well, as main process will exit as soon as it gets SIGCHLD and -d will exit as soon as ipc_wait_for_complete returns. (It's just less likely to be a problem as it's still sleep(1)-based, so the window for the race is much smaller) Feel free to squash with the previous patch (swupdate -i: remove cleanup_files call in main()) instead if you prefer. core/stream_interface.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)