dhcpd6 prefixes script can fail to restart

[bensmithurst]

While investigating a problem with delegated prefixes not getting a route set to the client which received the prefix I discovered that the problem was caused by the dhcp/prefixes.sh script not running.

When I checked the system log I saw messages like this:

[meta sequenceId="54"] /usr/local/sbin/pluginctl: The command '/usr/sbin/daemon -f -p '/var/run/dhcpleases6.pid' '/usr/local/opnsense/scripts/dhcp/prefixes.sh'' returned exit code '3', the output was 'daemon: process already running, pid: 67377'

After a bit of digging I discovered it will often not cleanly restart due to the script itself being killed, but the daemon wrapper will wait for EOF on the pipe to the child process, and the sleep process called by the script also holds this pipe open. By redirecting sleep's stdout/stderr to /dev/null, it is no longer connected the pipe and will reliably restart.

I can reproduce the problem by doing sudo pluginctl -s dhcpd6 restart and checking if prefixes.sh appears in the process list. It will often fail to restart without this patch, and reliably restarts with the patch.

[fichtner]

Uh, thanks for the report/proposed fix. I'm wondering of adding set -e to the script instead of this might also do the trick?

[bensmithurst]

I will check later but off the top of my head I don't think that will work as that would deal with a command in the script exiting with a non-zero status, which isn't what's happening here - the problem is the script being killed while a child process is still alive.

[fichtner]

In theory the sleep should be interrupted causing a non-zero return code causing the script to stop.. if not we can also take your fix, although it will need a comment attached

[fichtner]

For the record, -f is supposed to close file descriptors to avoid such things. All a bit fishy.

[bensmithurst]

Added a comment, I'll do some more digging later - thanks for the pointers.

[bensmithurst]

daemon -m 0 would appear to prevent the script's stdout/stderr being piped to the parent process so that could be a better fix.

[fichtner]

@bensmithurst ok sounds good

[bensmithurst]

@fichtner I have updated the PR now with the daemon -m0 approach, which also works for me, and feels a bit cleaner than the sleep redirect.