From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH] apache: Fixes bug13656 - Add delay between stop & start of restart command. Date: Mon, 29 Apr 2024 21:44:31 +0200 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7505846515602763658==" List-Id: --===============7505846515602763658== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael, On 26/04/2024 19:08, Adolf Belka wrote: > Hi Michael, > > On 26/04/2024 17:30, Michael Tremer wrote: >> Hello, >> >>> On 26 Apr 2024, at 17:08, Adolf Belka wrote: >>> >>> Hi Michael, >>> >>> On 26/04/2024 16:45, Michael Tremer wrote: >>>> Hello Adolf, >>>> I don=E2=80=99t think that we should need this loop. I believe that /usr= /sbin/apachectl -k stop should only return once the service has properly been= terminated. Is that an incorrect assumption? >>> >>> Yes, I am afraid it is. >> >> Okay, looking at the source, you are correct. Apache only sends itself a S= IGTERM and that is it. >> >>> See my feedback in bug13657 from yesterday. >>> >>> I ran some tests with the new Apache script and if you have a lot of IPFi= re capabilities enabled then I found that in one test after running the stop = command and getting an OK the start command found a pid was already present s= o it didn't start Apache. However that pid was the old one (I recorded the pi= d before I ran the CU185 update), which was still hanging around when the sta= rt command was initiated. After not starting Apache because the old pid was s= till present that pid then disappeared as its removal finally occurred but th= en Apache was not running so the WUI froze. >>> >>> In the second test Apache gave the message "Address already in use" when = the start command was initiated and so again Apache was not started after the= stop. >>> >>> The only time I had the stop followed by start work was if I had an IPFir= e install with no other capabilities enabled such as Web Proxy, OpenVPN, IPS.= Then the update from CU184 to CU185 went smoothly without any freezing of th= e WUI and the Stop command was completed when the Start command started but n= ot when other IPFire capabilities were enabled. >> >> I just sent this patch to the list: https://patchwork.ipfire.org/project/i= pfire/patch/20240426152838.3768448-1-michael.tremer(a)ipfire.org/ >> >> This sets the PIDFILE variable so that killproc will wait for the main pro= cess to exit. This should do what you want to do without adding any custom co= de. >> >> Can you please test and confirm? > I saw that the change has been put into CU186 (next). I did an update from CU185 to CU186 in unstable and it went without any probl= ems so it looks like the patch change is working fine. Regards, Adolf. > I can't test that it fixes the problem with the update from CU184 to CU185.= If I put the changes into the Apache initscript in CU184 it will get overwri= tten by the shipping of the Apache initscript as it currently stands. > > The only test I could do is make sure it works by running the initscript wi= th the patch changes added, but I expect it will work. > > The only way I can test that effects I found yesterday no longer happen is = either by merging the change into CU185 or by removing the shipping of the Ap= ache initscript in CU185 until I have carried out the test. > > Regards, > > Adolf. > >> >> Best, >> -Michael >> >>> >>> Regards, >>> >>> Adolf. >>>> -Michael >>>>> On 24 Apr 2024, at 17:56, Adolf Belka wrote: >>>>> >>>>> - The change of the apache initscript in CU181 made the restart comand = a stop followed by a >>>>> =C2=A0=C2=A0 start. From bug13656 it looks like when the start is run t= he pid file may not yet be >>>>> =C2=A0=C2=A0 fully removed and so apache is not started, although the p= id is being removed. >>>>> - This patch checks if the pid file is still present every second up to= 10 seconds. Once the >>>>> =C2=A0=C2=A0 pid file is gone then the stop command is completed and th= e initscript moves to the start >>>>> =C2=A0=C2=A0 command. >>>>> - Rather than apply a fixed delay of 2 or 3 or 4 seconds I used a while= loop to check every >>>>> =C2=A0=C2=A0 second if the file is still present. If at the end of 10 s= econds it is still present >>>>> =C2=A0=C2=A0 then something went wrong with the pid removal. >>>>> - I have tested this patch on my vm system and it worked but it would b= e good to be >>>>> =C2=A0=C2=A0 reviewed to make sure that it is a reasonable approach tha= t has been used and if required >>>>> =C2=A0=C2=A0 changed in whatever way makes the best sense. >>>>> >>>>> Fixes: Bug13656 >>>>> Tested-by: Adolf Belka >>>>> Signed-off-by: Adolf Belka >>>>> --- >>>>> src/initscripts/system/apache | 12 +++++++++++- >>>>> 1 file changed, 11 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/src/initscripts/system/apache b/src/initscripts/system/apa= che >>>>> index 18eb86e2f..087e4084e 100644 >>>>> --- a/src/initscripts/system/apache >>>>> +++ b/src/initscripts/system/apache >>>>> @@ -2,7 +2,7 @@ >>>>> #######################################################################= ######## >>>>> # # >>>>> # IPFire.org - A linux based firewall=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # >>>>> -# Copyright (C) 2007-2022=C2=A0 IPFire Team =C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # >>>>> +# Copyright (C) 2007-2024=C2=A0 IPFire Team =C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # >>>>> # # >>>>> # This program is free software: you can redistribute it and/or modify= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # >>>>> # it under the terms of the GNU General Public License as published by= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # >>>>> @@ -87,6 +87,16 @@ case "$1" in >>>>> stop) >>>>> boot_mesg "Stopping Apache daemon..." >>>>> /usr/sbin/apachectl -k stop >>>>> + COUNTER=3D0 >>>>> + while [ -e /var/run/httpd.pid ] >>>>> + do >>>>> + sleep 1 >>>>> + (( COUNTER++ )) >>>>> + if [ $COUNTER -eq 10 ]; then >>>>> + boot_mesg "pid not removed after 10 seconds" >>>>> + break >>>>> + fi >>>>> + done >>>>> evaluate_retval >>>>> ;; >>>>> >>>>> --=20 >>>>> 2.44.0 >>>>> >> --===============7505846515602763658==--