Run cleanup handers first, then post processing #692
Conversation
Currently some post processing hooks are checks like memory check, if a worker hits a memory limit, it will try to recycle the worker. In that case, any post processing cleanup handling code wouldn't have a chance to run. This is to fix the execution order.
|
Hm, the comment suggests that when the proc manager exists that sends the TERM signal to workers and that is being handled in the cleanup handler. Isn't that the case you're talking about? |
|
Yes, when the manager exits, it sends its workers a TERM signal. That gets handled in the worker by the The cleanup handlers themselves have extra protection from a TERM signal as it's possible to either not have a manager at all. or to disable the signal handling in the manager. In either of those cases, the cleanup watches for that |
|
just curious, is there a reason not to move this after harakiri handling code either? |
|
At work we have some legacy hooks that happens in a ProcManager subclass' |
|
Can you supply a unit test that shows the original problem? |
|
I expect we can figure that out. |
|
Putting the discussion aside, you seem to be relying on some undocumented behaviors and ProcManager internals a lot. I'd fork the entire code into your own module and use it instead of using this, at least for now. I understand your desire to upstream the change and avoid a potential divergence, but this code has not been updated for years, so... |
|
We currently did fork and have an override with this change, but yes, would be much nicer to get it upstream and remove that. |
Manually set n_process=0 and call hanlding_init to avoid manager forking
…al handling Merge in GITHUB/plack from test-post-processing-order-fix to fix_post_processing_exe_order * commit '0b4648d13da7a2a1c90dce1dff0dba03e7ac293e': Update POD about cleanup hook Test cleanup handler and Proc Manager Term signal handling
|
You probably got a notification, but we pushed up a test for this. |
The FCGI::ProcManager
pm_post_dispatchmethod may exit if it got a signal. Currently that will exit before handling the cleanup handlers. This is similar to harakiri, and the PSGI::Extensions documentation suggests that "it SHOULD [be implemented] in a way that cleanup handlers run before harakiri". Moving the cleanup handlers before the call topm_post_dispatchshould behave more correctly.