[Buildroot] [PATCH v2 1/1] package/dnsmasq: Fix init script restart command

[Buildroot] [PATCH v2 1/1] package/dnsmasq: Fix init script restart command

[Fiona Klute via buildroot]

I have had constant issues restarting dnsmasq. Stop works, start fails
because the new instance can't bind the socket. Another restart
immediately after works just fine:

# /etc/init.d/S80dnsmasq restart
Stopping dnsmasq: OK
Starting dnsmasq:
dnsmasq: failed to create listening socket for 192.168.128.1: Address in use
FAIL
# /etc/init.d/S80dnsmasq restart
Stopping dnsmasq: FAIL
Starting dnsmasq: OK

Solve this by waiting for process to actually stop before returning
from the stop command. Clean up the PID file after to avoid potential
issues with the PID being reused after stop. The wait could also be
placed inside the restart block, but putting it into the stop block
has the advantage that it also avoids similar issues for any other
callers.

Signed-off-by: Fiona Klute <fiona.klute+wiwa@gmx.de>
---
Changes v1 -> v2:
Read $PIDFILE only once per iteration, to avoid errors if it
disappears between checks. Suggested by Yann E. MORIN.

 package/dnsmasq/S80dnsmasq | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/dnsmasq/S80dnsmasq b/package/dnsmasq/S80dnsmasq
index 175daf9d26..27163f5205 100644
--- a/package/dnsmasq/S80dnsmasq
+++ b/package/dnsmasq/S80dnsmasq
@@ -16,6 +16,13 @@ case "$1" in
 		printf "Stopping dnsmasq: "
 		start-stop-daemon -K -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON"
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
+		# wait for dnsmasq process to be gone
+		while true; do
+			pid="$( cat "${PIDFILE}" 2>/dev/null || true )"
+			( [ -n "${pid}" ] && [ -d "/proc/${pid}" ] ) || break
+			sleep 0.1
+		done
+		rm -f "$PIDFILE"
 		;;
 	restart|reload)
 		$0 stop
--
2.45.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/dnsmasq: Fix init script restart command

[Yann E. MORIN]

fiona, All,

On 2024-05-31 20:53 +0200, Fiona Klute via buildroot spake thusly:
> I have had constant issues restarting dnsmasq. Stop works, start fails

We try to have commit logs with a neutral tone, i.e. not in the first
person, so I slightly rephrased that sentence. I also reflowed the
part with the example failure.

Also, see below...

[--SNIP--]
> diff --git a/package/dnsmasq/S80dnsmasq b/package/dnsmasq/S80dnsmasq
> index 175daf9d26..27163f5205 100644
> --- a/package/dnsmasq/S80dnsmasq
> +++ b/package/dnsmasq/S80dnsmasq
> @@ -16,6 +16,13 @@ case "$1" in
>  		printf "Stopping dnsmasq: "
>  		start-stop-daemon -K -q -p "$PIDFILE" -x "/usr/sbin/$DAEMON"
>  		[ $? = 0 ] && echo "OK" || echo "FAIL"
> +		# wait for dnsmasq process to be gone
> +		while true; do
> +			pid="$( cat "${PIDFILE}" 2>/dev/null || true )"
> +			( [ -n "${pid}" ] && [ -d "/proc/${pid}" ] ) || break

I have a hook that runs check-paakcage without the exclusion list, and
that also runs shellcheck:

    $ ./utils/docker-run shellcheck package/dnsmasq/S80dnsmasq
    In package/dnsmasq/S80dnsmasq line 22:
                            ( [ -n "${pid}" ] && [ -d "/proc/${pid}" ] ) || break
                            ^-- SC2235: Use { ..; } instead of (..) to avoid subshell overhead.

So I fixed that (as well as the two other warnings that are unrelated to
your change), and applied to master, thanks.

Regards,
Yann E. MORIN.

> +			sleep 0.1
> +		done
> +		rm -f "$PIDFILE"
>  		;;
>  	restart|reload)
>  		$0 stop
> --
> 2.45.1
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/dnsmasq: Fix init script restart command

[Peter Korsgaard]

>>>>> "Fiona" == Fiona Klute via buildroot <buildroot@buildroot.org> writes:

 > I have had constant issues restarting dnsmasq. Stop works, start fails
 > because the new instance can't bind the socket. Another restart
 > immediately after works just fine:

 > # /etc/init.d/S80dnsmasq restart
 > Stopping dnsmasq: OK
 > Starting dnsmasq:
 > dnsmasq: failed to create listening socket for 192.168.128.1: Address in use
 > FAIL
 > # /etc/init.d/S80dnsmasq restart
 > Stopping dnsmasq: FAIL
 > Starting dnsmasq: OK

 > Solve this by waiting for process to actually stop before returning
 > from the stop command. Clean up the PID file after to avoid potential
 > issues with the PID being reused after stop. The wait could also be
 > placed inside the restart block, but putting it into the stop block
 > has the advantage that it also avoids similar issues for any other
 > callers.

 > Signed-off-by: Fiona Klute <fiona.klute+wiwa@gmx.de>
 > ---
 > Changes v1 -> v2:
 > Read $PIDFILE only once per iteration, to avoid errors if it
 > disappears between checks. Suggested by Yann E. MORIN.

Committed to 2024.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot