| Message ID | 20220509000728.767166-1-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] install_from_file: initialize and take mutex earlier | expand |
On 09.05.22 02:07, 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 > > Mymutex is not a descriptive variable name and has been renamed to > install_file_mutex, as it is barely used this was left in same patch. > > Also, making the error case return path shared also fixes 'close(fd)' > that should have been checking for filename being set. > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > v1 -> v2: > - drop mutex in error case, using a goto drives-by fixes the close that > was incorrect > - rename mymutex to install_file_mutex as it's a trivial change. > > core/install_from_file.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/core/install_from_file.c b/core/install_from_file.c > index 2230a4f88f3b..9614170ba8d7 100644 > --- a/core/install_from_file.c > +++ b/core/install_from_file.c > @@ -18,7 +18,7 @@ > #include "util.h" > #include "installer.h" > > -static pthread_mutex_t mymutex; > +static pthread_mutex_t install_file_mutex; > > static char buf[16 * 1024]; > static int fd = STDIN_FILENO; > @@ -63,9 +63,9 @@ static int endupdate(RECOVERY_STATUS status) > } > } > > - pthread_mutex_lock(&mymutex); > + pthread_mutex_lock(&install_file_mutex); > pthread_cond_signal(&cv_end); > - pthread_mutex_unlock(&mymutex); > + pthread_mutex_unlock(&install_file_mutex); > > return 0; > } > @@ -88,6 +88,8 @@ int install_from_file(const char *filename, bool check) > if (check) > req.dry_run = RUN_DRYRUN; > > + pthread_mutex_init(&install_file_mutex, NULL); > + pthread_mutex_lock(&install_file_mutex); > while (timeout_cnt > 0) { > rc = swupdate_async_start(readimage, NULL, > endupdate, &req, sizeof(req)); > @@ -100,16 +102,16 @@ int install_from_file(const char *filename, bool check) > /* return if we've hit an error scenario */ > if (rc < 0) { > ERROR ("swupdate_async_start returns %d\n", rc); > - close(fd); > - return EXIT_FAILURE; > + end_status = EXIT_FAILURE; > + goto out; > } > > - pthread_mutex_init(&mymutex, NULL); > > /* Now block */ > - pthread_mutex_lock(&mymutex); > - pthread_cond_wait(&cv_end, &mymutex); > - pthread_mutex_unlock(&mymutex); > + pthread_cond_wait(&cv_end, &install_file_mutex); > + > +out: > + pthread_mutex_unlock(&install_file_mutex); > > if (filename) > close(fd); Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/core/install_from_file.c b/core/install_from_file.c index 2230a4f88f3b..9614170ba8d7 100644 --- a/core/install_from_file.c +++ b/core/install_from_file.c @@ -18,7 +18,7 @@ #include "util.h" #include "installer.h" -static pthread_mutex_t mymutex; +static pthread_mutex_t install_file_mutex; static char buf[16 * 1024]; static int fd = STDIN_FILENO; @@ -63,9 +63,9 @@ static int endupdate(RECOVERY_STATUS status) } } - pthread_mutex_lock(&mymutex); + pthread_mutex_lock(&install_file_mutex); pthread_cond_signal(&cv_end); - pthread_mutex_unlock(&mymutex); + pthread_mutex_unlock(&install_file_mutex); return 0; } @@ -88,6 +88,8 @@ int install_from_file(const char *filename, bool check) if (check) req.dry_run = RUN_DRYRUN; + pthread_mutex_init(&install_file_mutex, NULL); + pthread_mutex_lock(&install_file_mutex); while (timeout_cnt > 0) { rc = swupdate_async_start(readimage, NULL, endupdate, &req, sizeof(req)); @@ -100,16 +102,16 @@ int install_from_file(const char *filename, bool check) /* return if we've hit an error scenario */ if (rc < 0) { ERROR ("swupdate_async_start returns %d\n", rc); - close(fd); - return EXIT_FAILURE; + end_status = EXIT_FAILURE; + goto out; } - pthread_mutex_init(&mymutex, NULL); /* Now block */ - pthread_mutex_lock(&mymutex); - pthread_cond_wait(&cv_end, &mymutex); - pthread_mutex_unlock(&mymutex); + pthread_cond_wait(&cv_end, &install_file_mutex); + +out: + pthread_mutex_unlock(&install_file_mutex); if (filename) close(fd);
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 Mymutex is not a descriptive variable name and has been renamed to install_file_mutex, as it is barely used this was left in same patch. Also, making the error case return path shared also fixes 'close(fd)' that should have been checking for filename being set. Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- v1 -> v2: - drop mutex in error case, using a goto drives-by fixes the close that was incorrect - rename mymutex to install_file_mutex as it's a trivial change. core/install_from_file.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)