Message ID | 20211012164240.3546-1-stefan.schantl@ipfire.org |
---|---|
State | Deferred |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384 client-signature ECDSA (P-384) client-digest SHA384) (Client CN "mail01.haj.ipfire.org", Issuer "R3" (verified OK)) by web04.haj.ipfire.org (Postfix) with ESMTPS id 4HTM0W5fd9z3wcZ for <patchwork@web04.haj.ipfire.org>; Tue, 12 Oct 2021 16:42:51 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384 client-signature ECDSA (P-384) client-digest SHA384) (Client CN "mail02.haj.ipfire.org", Issuer "R3" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4HTM0V2z0xzsv; Tue, 12 Oct 2021 16:42:50 +0000 (UTC) Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4HTM0V18MSz2xxL; Tue, 12 Oct 2021 16:42:50 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384 client-signature ECDSA (P-384) client-digest SHA384) (Client CN "mail01.haj.ipfire.org", Issuer "R3" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4HTM0T1tplz2xks for <development@lists.ipfire.org>; Tue, 12 Oct 2021 16:42:49 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4HTM0S1kfBzcg; Tue, 12 Oct 2021 16:42:48 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1634056968; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=ZC05U8wYEykba3xfbLZ5X1OrgHNfNhFdMG5cf5QaMnU=; b=IIC2SRMAyGblcKwbCShywiekZPaTcF62hD69949FN6hs3rSBgAWDUMmXz2bHRVaQ+8ujzR Hr0eve7a5bhO7QCg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1634056968; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=ZC05U8wYEykba3xfbLZ5X1OrgHNfNhFdMG5cf5QaMnU=; b=MQkiDqLds20OR1SyF3hgErFV+Bzw+kvx4SOFPKxMyxbefgxOGKvCZPcRi0p/6X5QvslRH+ tO3cTwCaF05k8i2/bNVOiceQtSNLRJqxeGYymp5OoUWKOAT6PCceJRyO7xevYr3scMHTJg Cq/s/kpodQo+Qbtha29JT9k9L0h91BR+IVjGAQ0XHkhJWbgWoIv772stVqHoqv4rld7PL9 m74KgFPwJA5lBRq6NTzOk6sEFlu9kztbp0O02t/u0OH5A4UddQGq619W2Qu+rir7lhZAM8 wX+n+Q48Uno3AWV8LY16Uw7oyMlRxHrP+6ufdhkj2Abl0sEf18Q9BKNjTAFeqg== From: Stefan Schantl <stefan.schantl@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH] pakfire.cgi: Give pakfire time to write lockfile when update/upgrade. Date: Tue, 12 Oct 2021 18:42:40 +0200 Message-Id: <20211012164240.3546-1-stefan.schantl@ipfire.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <https://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <https://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Series |
pakfire.cgi: Give pakfire time to write lockfile when update/upgrade.
|
|
Commit Message
Stefan Schantl
Oct. 12, 2021, 4:42 p.m. UTC
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@ipfire.org>
---
html/cgi-bin/pakfire.cgi | 2 ++
1 file changed, 2 insertions(+)
Comments
Acked-by: Peter Müller <peter.mueller@ipfire.org> > 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@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"}; > >
Reviewed-by: Michael Tremer <michael.tremer@ipfire.org> > On 12 Oct 2021, at 18:23, Peter Müller <peter.mueller@ipfire.org> wrote: > > Acked-by: Peter Müller <peter.mueller@ipfire.org> > >> 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@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"}; >> >>
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@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"}; >
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@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@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"}; >> > >
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@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@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@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"}; > >>
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"};