Re: [PATCH] pakfire.cgi: Give pakfire time to write lockfile when update/upgrade.

[Tapani Tarvainen <ipfire@tapanitarvainen.fi>]

[-- Attachment #1: Type: text/plain, Size: 3471 bytes --]

How about a simple timestamp test?

Have pakfire touch something when it has created its lockfile, then
the calling script could check its timestamp before executing
system_background() and afterwards check if it has changed.

That should be easy enough to implement.

Tapani

On Fri, Oct 15, 2021 at 11:14:09AM +0100, Michael Tremer (michael.tremer(a)ipfire.org) wrote:
> 
> Hello,
> 
> This is a classic race condition, but I am not sure if there is a more elegant way that can be implemented very simply.
> 
> The problem presumably is that system_background() is too fast. It forks first and then launches pakfire. The behaviour was different previously. We started a shell which then launched pakfire and we waited until that shell has returned.
> 
> If anything, sleeping longer will increase your chances to only continue after pakfire has been launched.
> 
> Anything that checks whether pakfire’s lock file shows up is probably not a good solution because it might run indefinitely when pakfire finishes its job very quickly.
> 
> With the right checks around it, this might be our only option unless we want to create a more complicated inter-process communication system for this.
> 
> -Michael
> 
> > On 15 Oct 2021, at 10:33, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> > 
> > Hello List,
> > 
> > after some more testing I had to decide to reject this patch from
> > beeing merged.
> > 
> > It simply does not work all the time, because may the sleep time is too
> > short if the system is busy, or pakfire needs to long to startup and
> > place the lockfile.
> > 
> > So I have to go back to the drawing board and do a better solution for
> > this issue.
> > 
> > Sorry for the noise on the list,
> > 
> > -Stefan
> > 
> >> In case the package list should be grabbed or the system should be
> >> upgraded, pakfire got called which writes a lock file to prevent from
> >> beeing launched multiple times and to lock the pakfire.cgi with the
> >> nice
> >> log output.
> >> 
> >> In case update or upgrade has been performed via WUI, pakfire has
> >> been called
> >> and written the file in the background but the WUI script has been
> >> executed further and because of a race condition it did not recognize
> >> the lockfile at this moment because it was not present.
> >> 
> >> So a simple sleep should to the trick and give pakfire the required
> >> time
> >> to write out it's lockfile.
> >> 
> >> Fixes #12696.
> >> 
> >> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> >> ---
> >>  html/cgi-bin/pakfire.cgi | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> >> index 0cf522ba1..aaf63d469 100644
> >> --- a/html/cgi-bin/pakfire.cgi
> >> +++ b/html/cgi-bin/pakfire.cgi
> >> @@ -133,8 +133,10 @@ END
> >>  
> >>  } elsif (($cgiparams{'ACTION'} eq 'update') && (! -e
> >> $Pakfire::lockfile)) {
> >>         &General::system_background("/usr/local/bin/pakfire",
> >> "update", "--force", "--no-colors");
> >> +       sleep(1);
> >>  } elsif (($cgiparams{'ACTION'} eq 'upgrade') && (!-e
> >> $Pakfire::lockfile)) {
> >>         &General::system_background("/usr/local/bin/pakfire",
> >> "upgrade", "-y", "--no-colors");
> >> +       sleep(1);
> >>  } elsif ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}") {
> >>         $pakfiresettings{"TREE"} = $cgiparams{"TREE"};
> >>