Potential patch to address #28

[captbrando]

Close issue #28

[ianharrier]

Thank you for your contribution!

I think the use case you outlined in #28 is certainly valid. I'm in favor of merging in a timeout functionality. I'd want this to be an opt-in feature (i.e. by default, timeout should not be used), as this could potentially be a breaking change for some people. Let's leave the timeout variable unset by default, and only use the timeout command if the variable is set.

Also, let's call the variable RECONNECT_TIMEOUT, instead of KILL_TIMEOUT, for clarity. I know you left a question in #28 concerning whether a timeout would be helpful in any other portions of the script. If a use case comes up in the future, we can have a separate timeout variable for that.

Thanks again!

[captbrando]

After further back and forth with the maintainer of timeout, this is the correct functionality. The first argument after -k is the seconds to wait after a SIGTERM is sent to send SIGKILL, not the additional signal. So I think the second commit is the correct one for that line.

[ianharrier]

I just read through the bug report you opened on timeout. Based on the last comment, and based on timeout --help (included below)…

Usage: timeout [OPTION] DURATION COMMAND [ARG]...
  or:  timeout [OPTION]
Start COMMAND, and kill it if still running after DURATION.

Mandatory arguments to long options are mandatory for short options too.
      --preserve-status
                 exit with the same status as COMMAND, even when the
                   command times out
      --foreground
                 when not running timeout directly from a shell prompt,
                   allow COMMAND to read from the TTY and get TTY signals;
                   in this mode, children of COMMAND will not be timed out
  -k, --kill-after=DURATION
                 also send a KILL signal if COMMAND is still running
                   this long after the initial signal was sent
  -s, --signal=SIGNAL
                 specify the signal to be sent on timeout;
                   SIGNAL may be a name like 'HUP' or a number;
                   see 'kill -l' for a list of signals
  -v, --verbose  diagnose to stderr any signal sent upon timeout
      --help        display this help and exit
      --version     output version information and exit

DURATION is a floating point number with an optional suffix:
's' for seconds (the default), 'm' for minutes, 'h' for hours or 'd' for days.
A duration of 0 disables the associated timeout.

Upon timeout, send the TERM signal to COMMAND, if no other SIGNAL specified.
The TERM signal kills any process that does not block or catch that signal.
It may be necessary to use the KILL signal, since this signal can't be caught.
...

…the syntax of the command should be timeout [OPTION] DURATION COMMAND [ARG].... Or in our case, timeout [OPTION] $KILL_TIMEOUT /usr/syno/bin/synovpnc connect --id=$PROFILE_ID. If you're wanting to explicitly set the signal to SIGKILL, it seems like we should use the -s option rather than the -k option, yielding the following:

/bin/timeout -s 9 $KILL_TIMEOUT /usr/syno/bin/synovpnc connect --id=$PROFILE_ID

Additionally, testing should be done to determine if SIGTERM works instead of SIGKILL, which would allow us to eliminate the -s option alltogether, as the timeout command sends SIGTERM by default, yielding the following:

/bin/timeout $KILL_TIMEOUT /usr/syno/bin/synovpnc connect --id=$PROFILE_ID

Please let me know what you find. Thanks!

[captbrando]

It's either/or. So the -k flag is a shortcut that would allow synovpnc to exit gracefully under a SIGTERM first, then a SIGKILL after a certain amount of seconds after. For simplicity, we could use -s and just bypass SIGTERM.

So, I would argue, perhaps the best option is to either reuse the $KILL_TIMEOUT variable twice, or make a second variable like $GRACEFUL_TIMEOUT that would be used to send SIGTERM. The line would look like:

/bin/timeout -k $KILL_TIMEOUT $KILL_TIMEOUT /usr/syno/bin/synovpnc connect --id=$PROFILE_ID

or

/bin/timeout -k $KILL_TIMEOUT $GRACEFUL_TIMEOUT /usr/syno/bin/synovpnc connect --id=$PROFILE_ID

If $KILL_TIMEOUT is set to 10s, the first line would wait 10s, send a SIGTERM, and if the process did not quit 10s later, it would send a SIGKILL. The second would tell timeout to wait $GRACEFUL_TIMEOUT duration before sending the SIGTERM, and if the process didn't gracefully exit within the $KILL_TIMEOUT duration, it would then send the SIGKILL.

Thanks again for considering. I now know way more about timeout than I ever really intended!