OpenVPN
Script timeout -- why have them?
The scripts run by the GUI (pre-connect, connect and disconnect) have associated timeout values in the registry. Out of these only the connect script timeout is used in any real sense (to report a timeout error that is ambiguous), the rest are just used for sleeping for that many seconds for no apparent reason.
I would like to clean up these, and run the scripts in their own threads, but wonder why the timeouts are there in the first place. The wait in worker thread just causes the status window to go non-responsive, especially for scripts that may take several seconds to complete. None of the scripts are force-terminated after timeout so it seems we can get rid of those waits altogether. Or increase those to fairly large values to be used to terminate run-away scripts...
Any thoughs?
Maybe the pre-connect and disconnect script timeouts are there just because of symmetry ("connect scripts have timeout, so these should too". Force-termination of scripts, coupled with a high default timeout value that is user-configurable makes sense in my opinion.
why to have timeouts at both openvpn (config files) and openvpn-gui? are there cases which cannot be resolved with timeouts specified at openvpn config files?
Took a while to get back to this topic. Thanks for the comments. @chipitsine : this is for scripts run directly by the GUI which is different from up/down scripts run by openvpn.
I think we can improve the timeout handling as below:
- preconnect_script: keep default timeout small (10s?), do not connect until script exits or times out, kill if still running, warn the user and proceed with the connection. The assumption is that the user wants this script to complete before connecting.
- connect_script: have a long default timeout, let the script run in a separate thread and let die on timeout. timeout=0 interpreted as infinite (i.e. never kill). The idea is that this may involve tasks like mapping shared drives which can sometimes take a while but the GUI need not wait for it to complete.
- disconnect_script: like the preconnect script this has to complete before disconnection so keep a small default timeout (10s?) and kill if not complete and then process the disconnection.
The main difference from the current behaviour would be (i) pre-connect and disconnect scripts are killed on timeout and (ii) connect script is not waited on. Also make sure windows events are handled during the wait so that the GUI doesn't appear to hang during the wait as it does now.
In the first and last cases the users who do not want the script to be killed can put the task into background and return promptly.
If this sounds sensible I'll make a patch.
@selvanair : as long as failed scripts create a warning this should improve things from user perspective, so a +1 for a patch.
Reviving this long forgotten thread in light of the recent thread on the users ML: https://www.mail-archive.com/[email protected]/msg04367.html
In addition to what I had proposed we'd also need to treat preconnectscript failures as an error or have an option to do so.
Taking that email thread into account .. I think the pre-connect script should be able to terminate the attempt by the GUI to launch the openvpn binary with what ever config. (That does not appear to have been made clear so far)
Yep. Had forgotten that crucial detail until I went back read what I wrote in this thread a long time ago :)