Cleanup signal handling for agents and enable them earlier. #2571
Conversation
The motivation for this is that during policy loading, certain functions may be evaluated that open pipes, and if these pipes at any point become disconnected, we get an EPIPE signal. This is ignored once we get to the evaluation stage, but during loading the signal handlers have not been enabled yet. This was originally seen during introduction of the mapdata() function, which opens a pipe to an external process. In the test for this function, `cat` was used to output a prerendered response, but `cat` was also expected to consume the data input. Depending on timing, if the data was written to `cat` before it terminated, everything would be fine, but if `cat` terminated before data could be written (because it was catting a file, not from stdin), cf-promises would get an EPIPE signal and terminate abnormally. Upon inspection of our signal handling, it appears this can happen in cf-agent as well, and is as such "a ticking time bomb". Therefore this commit cleans up the signal handling a bit, consolidates it where possible, and moves all signal handler initialization to the beginning of each agent main(). Daemons have other signal handler requirements and were left alone for now.
|
@jimis: I'd like your input on this one. |
|
Yeah, nice catch! |
|
Nice catch indeed! Will look into it deeper in a bit. :-) |
|
Makes sense. What I don't get is why So +1, I can't think of any side-effects this can have. |
|
Thanks for looking at it! I will merge it together with the PR that prompted this. |
|
I was under the impression that in theory only common bundles need to be evaluated in pre-eval. So if it's a "vars" promise in a non-common bundle, it should be skipped. |
|
No, all variables are resolved in the pre-eval step, regardless of which bundle they are in. |
Why would that happen? The variables in common bundles being pre-evaluated, makes sense in order to resolve dynamic inputs. But what would the reason be for that to happen in all bundles, during pre-evaluation? |
|
I don't know what the original motivation was, it just seems to be that way now. |
The motivation for this is that during policy loading, certain
functions may be evaluated that open pipes, and if these pipes at any
point become disconnected, we get an EPIPE signal. This is ignored
once we get to the evaluation stage, but during loading the signal
handlers have not been enabled yet.
This was originally seen during introduction of the mapdata()
function, which opens a pipe to an external process. In the test for
this function,
catwas used to output a prerendered response, butcatwas also expected to consume the data input. Depending ontiming, if the data was written to
catbefore it terminated,everything would be fine, but if
catterminated before data could bewritten (because it was catting a file, not from stdin), cf-promises
would get an EPIPE signal and terminate abnormally.
Upon inspection of our signal handling, it appears this can happen in
cf-agent as well, and is as such "a ticking time bomb". Therefore this
commit cleans up the signal handling a bit, consolidates it where
possible, and moves all signal handler initialization to the beginning
of each agent main().
Daemons have other signal handler requirements and were left alone for
now.