Fix #494 sshuttle caught in infinite select() loop. #520
Conversation
Improve detection of when the ssh process exits in both daemon and foreground modes. Previously, sshuttle could infinite loop with 100% cpu usage if the ssh process died. On machines that use suspend, the ssh connection might not resume after wakeup. Now, this situation is detected and sshuttle exits. The fix involves changing the return value we check for when we call poll() and using a psutil function to detect when the process exits if we are running sshuttle as a daemon.
|
Thanks for this. So if I understand this correctly, the problem is that we are creating the ssh connection before calling Shame this requires another dependency, but at least it is client only. Wonder if it is possible to convince codacy to be happy. I can't see anything obvious... |
|
As far as I can tell, yes. The ssh connection gets setup as a child process. Then when daemonize() gets called, the main thread becomes a new process---but the ssh process's parent is still the original process prior to daemonize(). In this case, calling poll() from the new process on the ssh "child" (which is no longer our child) apparently always returns 0. I spent a bit of time trying to figure out if there was another reliable way besides psutil to see if there was a PID running across different platforms, but found none. Also, it didn't seem that we could start ssh after daemonize. There wasn't another obvious way to detect that ssh was dead. The problem also occurred when running in the foreground for a different reason: poll() would return None when the process is still running, but then can return 0 when ssh exits...but the original logic failed to catch that with just "if rv:". If anybody wants to poke at the problem or reproduce, do the following on the client machine: start sshuttle (without this patch), use "ps aux | grep ssh" to find the ssh process that sshuttle started, kill that ssh process, and then open a socket that would normally go through sshuttle. CPU usage should spike to 100% on one core. As far as I can tell, this fix works reliably---but it could probably use additional eyeballs to confirm. The only "hiccup" with this fix is that the first socket you make that causes sshuttle to realize that ssh is dead results in some kind of connection error. But, after that, everything returns to as it was prior to running sshuttle. |
2 participants
Improve detection of when the ssh process exits in both daemon and
foreground modes. Previously, sshuttle could infinite loop with 100%
cpu usage if the ssh process died. On machines that use suspend, the
ssh connection might not resume after wakeup. Now, this situation is
detected and sshuttle exits. The fix involves changing the return
value we check for when we call poll() and using a psutil function to
detect when the process exits if we are running sshuttle as a daemon.