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: Fri, 26 Apr 2024 19:08:16 +0200 Message-ID: In-Reply-To: <01939645-1672-44D5-9933-7347CF772FD0@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6254821325896504678==" List-Id: --===============6254821325896504678== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael, On 26/04/2024 17:30, Michael Tremer wrote: > Hello, >=20 >> 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. >=20 > Okay, looking at the source, you are correct. Apache only sends itself a SI= GTERM and that is it. >=20 >> See my feedback in bug13657 from yesterday. >> >> I ran some tests with the new Apache script and if you have a lot of IPFir= e capabilities enabled then I found that in one test after running the stop c= ommand and getting an OK the start command found a pid was already present so= it didn't start Apache. However that pid was the old one (I recorded the pid= before I ran the CU185 update), which was still hanging around when the star= t command was initiated. After not starting Apache because the old pid was st= ill present that pid then disappeared as its removal finally occurred but the= n Apache was not running so the WUI froze. >> >> In the second test Apache gave the message "Address already in use" when t= he 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 IPFire= 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 the= WUI and the Stop command was completed when the Start command started but no= t when other IPFire capabilities were enabled. >=20 > I just sent this patch to the list: https://patchwork.ipfire.org/project/ip= fire/patch/20240426152838.3768448-1-michael.tremer(a)ipfire.org/ >=20 > This sets the PIDFILE variable so that killproc will wait for the main proc= ess to exit. This should do what you want to do without adding any custom cod= e. >=20 > Can you please test and confirm? I can't test that it fixes the problem with the update from CU184 to CU185. I= f I put the changes into the Apache initscript in CU184 it will get overwritt= en 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 with= 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 ei= ther by merging the change into CU185 or by removing the shipping of the Apac= he initscript in CU185 until I have carried out the test. Regards, Adolf. >=20 > Best, > -Michael >=20 >> >> 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 >>>> start. From bug13656 it looks like when the start is run the pid file= may not yet be >>>> fully removed and so apache is not started, although the pid is being= removed. >>>> - This patch checks if the pid file is still present every second up to = 10 seconds. Once the >>>> pid file is gone then the stop command is completed and the initscrip= t moves to the start >>>> command. >>>> - Rather than apply a fixed delay of 2 or 3 or 4 seconds I used a while = loop to check every >>>> second if the file is still present. If at the end of 10 seconds it i= s still present >>>> then something went wrong with the pid removal. >>>> - I have tested this patch on my vm system and it worked but it would be= good to be >>>> reviewed to make sure that it is a reasonable approach that has been = used and if required >>>> 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/apac= he >>>> index 18eb86e2f..087e4084e 100644 >>>> --- a/src/initscripts/system/apache >>>> +++ b/src/initscripts/system/apache >>>> @@ -2,7 +2,7 @@ >>>> ########################################################################= ####### >>>> # = # >>>> # IPFire.org - A linux based firewall = # >>>> -# Copyright (C) 2007-2022 IPFire Team = # >>>> +# Copyright (C) 2007-2024 IPFire Team = # >>>> # = # >>>> # This program is free software: you can redistribute it and/or modify = # >>>> # it under the terms of the GNU General Public License as published by = # >>>> @@ -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 >>>> >=20 --===============6254821325896504678==--