| Message ID | 20220422235944.2808227-5-dominique.martinet@atmark-techno.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/6] sigchld_handler: report child exit status correctly | expand |
On 23.04.22 01:59, Dominique Martinet wrote: > if async_thread finishes really fast, it's possible it'd try to call end > callback to signal us before we're waiting for it, leaving us hanging here. > > This completes the previous patch for swupdate -i > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > > (side-comment: shall we use that occasion to rename 'mymutex' ?) > > Note I didn't try to reproduce this bug, but I've seen it on other programs > so I'm sure this is wrong. I actually did that change first so I didn't try > the previous patch without this. > > (Thinking about it again, it's almost impossible to hit right now > because the current async_thread sleeps 1s before exiting whatever > happens, so we have 1s to get to that lock initialization/cond wait, > but that doesn't make this correct) > > core/install_from_file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/core/install_from_file.c b/core/install_from_file.c > index 2230a4f88f3b..66b66372c7cd 100644 > --- a/core/install_from_file.c > +++ b/core/install_from_file.c > @@ -88,6 +88,8 @@ int install_from_file(const char *filename, bool check) > if (check) > req.dry_run = RUN_DRYRUN; > > + pthread_mutex_init(&mymutex, NULL); > + pthread_mutex_lock(&mymutex); > while (timeout_cnt > 0) { > rc = swupdate_async_start(readimage, NULL, > endupdate, &req, sizeof(req)); > @@ -104,10 +106,8 @@ int install_from_file(const char *filename, bool check) > return EXIT_FAILURE; > } > > - pthread_mutex_init(&mymutex, NULL); > > /* Now block */ > - pthread_mutex_lock(&mymutex); > pthread_cond_wait(&cv_end, &mymutex); > pthread_mutex_unlock(&mymutex); > Reviewed-by: Stefano babic <sbabic@denx.de> Best regards, Stefano Babic
diff --git a/core/install_from_file.c b/core/install_from_file.c index 2230a4f88f3b..66b66372c7cd 100644 --- a/core/install_from_file.c +++ b/core/install_from_file.c @@ -88,6 +88,8 @@ int install_from_file(const char *filename, bool check) if (check) req.dry_run = RUN_DRYRUN; + pthread_mutex_init(&mymutex, NULL); + pthread_mutex_lock(&mymutex); while (timeout_cnt > 0) { rc = swupdate_async_start(readimage, NULL, endupdate, &req, sizeof(req)); @@ -104,10 +106,8 @@ int install_from_file(const char *filename, bool check) return EXIT_FAILURE; } - pthread_mutex_init(&mymutex, NULL); /* Now block */ - pthread_mutex_lock(&mymutex); pthread_cond_wait(&cv_end, &mymutex); pthread_mutex_unlock(&mymutex);
if async_thread finishes really fast, it's possible it'd try to call end callback to signal us before we're waiting for it, leaving us hanging here. This completes the previous patch for swupdate -i Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- (side-comment: shall we use that occasion to rename 'mymutex' ?) Note I didn't try to reproduce this bug, but I've seen it on other programs so I'm sure this is wrong. I actually did that change first so I didn't try the previous patch without this. (Thinking about it again, it's almost impossible to hit right now because the current async_thread sleeps 1s before exiting whatever happens, so we have 1s to get to that lock initialization/cond wait, but that doesn't make this correct) core/install_from_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)