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

Register the TSTP signal handler before invoking the first job #10

Closed
wants to merge 1 commit into from

Conversation

mdkent
Copy link

@mdkent mdkent commented May 30, 2018

Fixes an issue where if the first job is a long running one, and you issue a SIGQUIT to the resque-pool-manager for a gracefully shutdown, you'll end up with your worker in a stopped state (T) as the trap() hasn't been set. eg:

root       941 85.5  0.7 392348 211224 ?       Sl+  18:34   0:08  \_ resque-pool-master[7bba15]: managing [949]
root       949  3.0  0.7 459940 200488 ?       Sl   18:34   0:00      \_ resque-1.26.0: Forked 957 at 1527705279
root       957  0.0  0.7 459940 199704 ?       Sl   18:34   0:00          \_ resque-1.26.0: Processing demo since 1527705279 [LongJob]

$ kill -QUIT 941

root       941 47.5  0.7 392348 211224 ?       Sl+  18:34   0:08  \_ resque-pool-master[7bba15]: managing [949]
root       949  0.3  0.7 459940 200488 ?       Sl   18:34   0:00      \_ resque-1.26.0: Forked 957 at 1527705279
root       957  0.0  0.7 459940 199704 ?       Tl   18:34   0:00          \_ resque-1.26.0: Processing demo since 1527705279 [LongJob]

Oops! It stays wedged like this indefinitely.

@nevans nevans mentioned this pull request Feb 7, 2019
@nevans
Copy link

nevans commented Feb 7, 2019

I had some issues (e.g. after_fork hooks not getting called when they should) when I tried to simply move hijack_fork above perform. I fixed the tests in #11, and fixed the TSTP bug (plus tests) in #13, by simply moving the trap (and nothing else) before the first perform.

@stulentsev
Copy link
Owner

@nevans Thanks for taking the time to contribute. Lemme have a coffee and review your PRs.

@stulentsev
Copy link
Owner

stulentsev commented Feb 10, 2019

Fixed in #13

@stulentsev stulentsev closed this Feb 10, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants