dylanahsmith/resque — pull request #1 by wisq (2014-01-22 15:35:02+00:00)
Title: Unhandled errors in fork

This is my best guess as to what's happening in Shopify/shopify#13133 .

Errors in `perform` are handled, but any errors that occur elsewhere — in `unregister_signal_handlers`, in `procline` (I/O errors?), in the block passed to `perform`, etc. — are propagated upwards, escaping the `exit!` and allowing the child to run rampant pretending to be a parent.

It doesn't get far, because it hits `unregister_worker` and tries to use Redis, only to discover it never reconnected Redis so it's screwed. That error then masks whatever the original error was.

It's vitally important that we never escape a `fork`. Hence, my first commit surrounds the inside of the fork block with a `begin..ensure`, with a `rescue` to try to helpfully output the error so we can at least confirm if this is the case.

My second commit (optional) tries to switch to the safer and more idiomatic (I think?) block version of fork, which is safe from escaping by its very nature. Take it or leave it at your discretion; it's the first one that really matters.

I'd love to provide tests for this, but a) I don't think the test suite tests forking, and b) the test suite is badly broken. Maybe it's out of date or something, but it's relying on a deprecated-then-removed Redis config directive (`glueoutputbuf`), an apparently nonexistent test method (`should_respond_with_success`), and even once I fix those, I get a ton of failures. So, I dunno. Test on staging, I guess?