Add missing bits of VRT_fail() handling during vcl_init {} / vcl_fini{}
This change is based on the precondition that VRT_fail() should work wherever we have a VRT_CTX and DTRT, see also previous. on vcl_init {} -------------- We already handled a "direct" VRT_fail() via a vmod function correctly. This commit adds a test. Yet we can also VRT_fail() via a PRIV_TASK fini callback and did not handle that case (triggered an assertion in VRT_fail()). Notice that this case has worked in client / backend tasks for long and a test case has been added with 746384b2 . It has now gained relevance with 43d9e5fb on vcl_fini {} -------------- By design, vcl_fini {} must not fail, which is also why the only valid return() value is "ok" (VCL_RET_OK). Consequently, we had if (method && *ctx->handling && *ctx->handling != VCL_RET_OK) { assert(ev == VCL_EVENT_LOAD); in vcl_send_event(). And then came VRT_fail(). If we wanted to sustain an assertion similar to the above, we would need to require vmod authors to never call VRT_fail() from VCL_MET_FINI - which would complicate code, be likely overlooked and receive little to no test coverage. So instead we now ignore errors during vcl_fini {} and emit a VCL_error log line. I considered the alternative to check for VCL_MET_FINI in VRT_fail(), but decided to handle the exception where it applies, which is in vcl_send_event(). I am aware that this part of the change may trigger some controversy and I am open to discussing alternatives. If anything, we now avoid unmotivated assertion failures triggered for the new tests in v00051.vtc for the time being.
Showing
Please register or sign in to comment