From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tapani Tarvainen To: development@lists.ipfire.org Subject: Re: [PATCH] pakfire.cgi: Give pakfire time to write lockfile when update/upgrade. Date: Fri, 15 Oct 2021 16:30:06 +0300 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3675713277064707343==" List-Id: --===============3675713277064707343== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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)ip= fire.org) wrote: >=20 > Hello, >=20 > This is a classic race condition, but I am not sure if there is a more eleg= ant way that can be implemented very simply. >=20 > The problem presumably is that system_background() is too fast. It forks fi= rst and then launches pakfire. The behaviour was different previously. We sta= rted a shell which then launched pakfire and we waited until that shell has r= eturned. >=20 > If anything, sleeping longer will increase your chances to only continue af= ter pakfire has been launched. >=20 > Anything that checks whether pakfire=E2=80=99s lock file shows up is probab= ly not a good solution because it might run indefinitely when pakfire finishe= s its job very quickly. >=20 > With the right checks around it, this might be our only option unless we wa= nt to create a more complicated inter-process communication system for this. >=20 > -Michael >=20 > > On 15 Oct 2021, at 10:33, Stefan Schantl wr= ote: > >=20 > > Hello List, > >=20 > > after some more testing I had to decide to reject this patch from > > beeing merged. > >=20 > > 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. > >=20 > > So I have to go back to the drawing board and do a better solution for > > this issue. > >=20 > > Sorry for the noise on the list, > >=20 > > -Stefan > >=20 > >> 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. > >>=20 > >> 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. > >>=20 > >> So a simple sleep should to the trick and give pakfire the required > >> time > >> to write out it's lockfile. > >>=20 > >> Fixes #12696. > >>=20 > >> Signed-off-by: Stefan Schantl > >> --- > >> html/cgi-bin/pakfire.cgi | 2 ++ > >> 1 file changed, 2 insertions(+) > >>=20 > >> 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 > >> =20 > >> } 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"} =3D $cgiparams{"TREE"}; > >> =20 --===============3675713277064707343==--