Refactor PIDFile.join()

[webknjaz]

This may be partially fixing #7.

[jaraco]

I'm a little unsure about this change. First thing is that join() used to be simple and very similar to wait(). After this change, it's much more complicated. Second is that join() used to have a simple function - wait until the file is gone. That fact that sometimes the file is gone and reappears during a check isn't necessarily the use-case that function was intended to serve.

But more importantly, I don't understand why the existing code is failing. When I run the tests on master with -k SIGHUP, it hangs on the very first test. The very first test is seeking to assert that the process exits on SIGHUP when not daemonized. But if there's still a pidfile after sending the signal, then the expected behavior isn't present. There's no race condition.

When I break into the process, I can see the test_signals.log looks like this:

[b'2020-03-11T20:16:27.584967'] (Bus 167dc5b8) Bus state: ENTER
[b'2020-03-11T20:16:27.585242'] (Bus 167dc5b8) PID 6409 written to '/Users/jaraco/code/public/cherrypy/magicbus/magicbus/test/test_signals.py.pid'.
[b'2020-03-11T20:16:27.585297'] (Bus 167dc5b8) Listening for SIGTERM.
[b'2020-03-11T20:16:27.585387'] (Bus 167dc5b8) Listening for SIGHUP.
[b'2020-03-11T20:16:27.585415'] (Bus 167dc5b8) Listening for SIGUSR1.
[b'2020-03-11T20:16:27.585435'] (Bus 167dc5b8) Bus state: IDLE
[b'2020-03-11T20:16:27.585453'] (Bus 167dc5b8) Bus state: START
[b'2020-03-11T20:16:27.585468'] (Bus 167dc5b8) Bus state: RUN
[b'2020-03-11T20:16:27.616095'] (Bus 167dc5b8) Caught signal SIGHUP.
[b'2020-03-11T20:16:27.616157'] (Bus 167dc5b8) SIGHUP caught while daemonized. Restarting.
[b'2020-03-11T20:16:27.616209'] (Bus 167dc5b8) Bus state: STOP
[b'2020-03-11T20:16:27.616234'] (Bus 167dc5b8) Bus state: IDLE
[b'2020-03-11T20:16:27.616254'] (Bus 167dc5b8) Bus state: EXIT
[b'2020-03-11T20:16:27.616398'] (Bus 167dc5b8) PID file removed: '/Users/jaraco/code/public/cherrypy/magicbus/magicbus/test/test_signals.py.pid'.
[b'2020-03-11T20:16:27.616423'] (Bus 167dc5b8) Waiting for child threads to terminate...
[b'2020-03-11T20:16:27.616457'] (Bus 167dc5b8) Bus state: EXITED
[b'2020-03-11T20:16:27.616480'] (Bus 167dc5b8) Re-spawning /Users/jaraco/code/public/cherrypy/magicbus/magicbus/test/test_signals.py tty
[b'2020-03-11T20:16:27.708287'] (Bus 5767f28d) Bus state: ENTER
[b'2020-03-11T20:16:27.708557'] (Bus 5767f28d) PID 6409 written to '/Users/jaraco/code/public/cherrypy/magicbus/magicbus/test/test_signals.py.pid'.
[b'2020-03-11T20:16:27.708601'] (Bus 5767f28d) Listening for SIGTERM.
[b'2020-03-11T20:16:27.708700'] (Bus 5767f28d) Listening for SIGHUP.
[b'2020-03-11T20:16:27.708730'] (Bus 5767f28d) Listening for SIGUSR1.
[b'2020-03-11T20:16:27.708753'] (Bus 5767f28d) Bus state: IDLE
[b'2020-03-11T20:16:27.708774'] (Bus 5767f28d) Bus state: START
[b'2020-03-11T20:16:27.708792'] (Bus 5767f28d) Bus state: RUN

It seems that at least on macOS, the asserted behavior just isn't happening. Sending SIGHUP is causing the process to restart and not shut down as expected.

[webknjaz]

It seems that at least on macOS, the asserted behavior just isn't happening. Sending SIGHUP is causing the process to restart and not shut down as expected.

That's correct. It shouldn't shut down. SIGTERM should shut down the process, but SIGHUP should cause reload/restart and that's what it does. When the process receives SIGHUP, it first goes to a stopped state (with a PID file removed) and then it starts again (creating a PID file in the same location).

The very first test is seeking to assert that the process exits on SIGHUP when not daemonized. But if there's still a pidfile after sending the signal, then the expected behavior isn't present. There's no race condition.

No, this test tries to catch a moment "in-between". When reload reached stop state but start hasn't created a PID file yet.
I confirmed that there's a race condition by using watch -n.1 'stat /path/to/file.pid' in a separate terminal while running the test. At the beginning, there's one file but when SIGHUP is sent it disappears for a bit (almost unnoticeable) and gets recreated with a new created timestamp.

That fact that sometimes the file is gone and reappears during a check isn't necessarily the use-case that function was intended to serve.

Right, I didn't think that it might be the original intent.

But more importantly, I don't understand why the existing code is failing. When I run the tests on master with -k SIGHUP, it hangs on the very first test.

Fun fact: if you add -s to pytest, it doesn't hang. Maybe it's a race condition between tests and subprocess.

[jaraco]

I'm afraid I don't have much more to add here. I've lost context on the issue and I was obviously struggling with it to begin with. I'm a little uneasy with the added complexity, but if it's the necessary complexity to be stable, I guess it's worth it.

I'll defer to your judgment.

[webknjaz]

I remember thinking that this felt like the way to go, in terms what the original intent seemed like. I guess I could double check and merge.