Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hupper & Twisted #11

Closed
hynek opened this issue Jan 11, 2017 · 10 comments
Closed

hupper & Twisted #11

hynek opened this issue Jan 11, 2017 · 10 comments

Comments

@hynek
Copy link

hynek commented Jan 11, 2017

Hi,

I’d like to use Twisted as a WSGI container for a Pyramid app and use hupper in dev.

Sadly there seems to be something broken about that setup when cleaning that I suspect is related to threads:

$ hupper -m twisted web --path=.
Starting monitor for PID 78003.
2017-01-11T17:46:46+0100 [-] Site starting on 8080
2017-01-11T17:46:46+0100 [twisted.web.server.Site#info] Starting factory <twisted.web.server.Site instance at 0x1021e5a28>
2017-01-11T17:46:46+0100 [twisted.application.runner._runner.Runner#info] Starting reactor...
^CKilling server with PID 78003.
2017-01-11T17:46:49+0100 [-] Received SIGTERM, shutting down.
2017-01-11T17:46:49+0100 [-] Received SIGINT, shutting down.
2017-01-11T17:46:49+0100 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/application/runner/_runner.py", line 63, in run
	    self.startReactor()
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/application/runner/_runner.py", line 166, in startReactor
	    reactor.run()
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/internet/base.py", line 1199, in run
	    self.mainLoop()
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/internet/base.py", line 1208, in mainLoop
	    self.runUntilCurrent()
	--- <exception caught here> ---
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/internet/base.py", line 801, in runUntilCurrent
	    f(*a, **kw)
	  File "/Users/hynek/.virtualenvs/tempenv-1c44245683697/lib/python2.7/site-packages/twisted/internet/base.py", line 584, in stop
	    "Can't stop reactor that isn't running.")
	twisted.internet.error.ReactorNotRunning: Can't stop reactor that isn't running.

2017-01-11T17:46:49+0100 [-] (TCP Port 8080 Closed)
2017-01-11T17:46:49+0100 [twisted.web.server.Site#info] Stopping factory <twisted.web.server.Site instance at 0x1021e5a28>
2017-01-11T17:46:49+0100 [-] Main loop terminated.

Please note I’m pressing Ctrl-C there, it’s not exploding or something.

To reproduce simply install twisted and hupper and run the command line from my example.

Any ideas how to fix that? Since Twisted doesn’t have own means for reloading, it would be great if hupper had first class support.

P.S. Tested on macOS Sierra with both Python 3.5.2 & 2.7.13.

@glyph
Copy link

glyph commented Jan 24, 2017

Partially, this is a (decade-old) bug in Twisted: https://twistedmatrix.com/trac/ticket/2265

@mmerickel
Copy link
Member

mmerickel commented Jan 24, 2017

I assume this is because the Ctrl-C goes to the entire process group and then hupper invokes worker.terminate() which sends the SIGTERM.

@mmerickel
Copy link
Member

As far as handling this more gracefully in hupper.. I imagine this would require catching the SIGINT, marking some state as expecting a crash, and then after a timeout sending the SIGTERM if it's still not dead.

@mmerickel
Copy link
Member

I thought about this a little bit more and I fail to see how this is hupper's problem. AFAICT it properly kills the subprocess. Also, reloading seems to work fine so this is only an issue when killing things and things seem to end up dead... just with an annoying warning. It seems to me twisted simply needs to deregister the signal handlers when it starts to shut down.

I'll be happy to re-open this and / or ponder any concerns if you still think otherwise!

@glyph
Copy link

glyph commented Jan 24, 2017

@mmerickel - It looks like it sends the process 2 signals in rapid succession - SIGTERM followed by SIGINT. While Twisted should handle this better, it seems like hupper really ought to send one signal and then wait at least a little while before sending a second one.

@glyph
Copy link

glyph commented Jan 24, 2017

(For what it's worth, "de-registering" signal handlers when it receives the first signal would either cause it to die instantly with no shutdown on the second signal, or to barf out a KeyboardInterrupt traceback instead of the one it currently does. I think you mean it should ignore the signal on repeated invocations?)

@mmerickel
Copy link
Member

@glyph I can't control the first signal. The second one is hupper terminating the worker which I don't disagree could be delayed a bit. When you Ctrl-C the shell automatically sends it to both processes. I'd have to install a SIGINT handler on hupper, which it currently doesn't have, in order to even know that this might be happening.

hupper could install a SIGINT handler and then give the worker some time to die before terminating it. If someone sent a kill -2 <hupper master pid> then it would not be sent to the child and you'd have to wait a few seconds for it to terminate. Does this sound like what you're expecting?

@glyph
Copy link

glyph commented Jan 24, 2017

Does this sound like what you're expecting?

Yes, exactly. (In the case described in the bug report here, python -m twisted would exit immediately, which means no second signal would be necessary.)

@mmerickel
Copy link
Member

Okay sounds good, I'll reopen this and try to find some time to work on it.

@mmerickel mmerickel reopened this Jan 24, 2017
@mmerickel
Copy link
Member

Pushed 0.4.2 with this fix. Thanks @glyph and @hynek, let me know if there's any lingering issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants