From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] pakfire.cgi: Give pakfire time to write lockfile when
 update/upgrade.
Date: Fri, 15 Oct 2021 11:14:09 +0100
Message-ID: <F63886C0-7876-4DBD-A1CC-A60B1E19CD19@ipfire.org>
In-Reply-To: <36de2347d06cdfd86b4a82e492e918ed26f60c1e.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============2361083793609862884=="
List-Id: <development.lists.ipfire.org>

--===============2361083793609862884==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

This is a classic race condition, but I am not sure if there is a more elegan=
t way that can be implemented very simply.

The problem presumably is that system_background() is too fast. It forks firs=
t and then launches pakfire. The behaviour was different previously. We start=
ed a shell which then launched pakfire and we waited until that shell has ret=
urned.

If anything, sleeping longer will increase your chances to only continue afte=
r pakfire has been launched.

Anything that checks whether pakfire=E2=80=99s 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> wrot=
e:
>=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 <stefan.schantl(a)ipfire.org>
>> ---
>>  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
>=20
>=20


--===============2361083793609862884==--