libpcp_web: fix libuv teardown; pmfind: defer context release and shutdown order#2557
libpcp_web: fix libuv teardown; pmfind: defer context release and shutdown order#2557kurik wants to merge 2 commits intoperformancecopilot:mainfrom
Conversation
…tdown order Add pmWebTimerLoopFinalize() to uv_close the global instrumentation timer after pmWebTimerClose and export it in PCP_WEB_1.24. Order pmWebGroupClose so struct webgroups is freed only from uv_close callbacks (timer then async) and clear module->privdata early to avoid double teardown. In pmfind, defer source_release to the next tick to avoid use-after-free when pmWebGroupDestroy runs from on_done; delete from contexts dict before destroy; track defer_pending and use extra uv_run/uv_stop as needed. Call pmWebGroupClose and pmWebTimerLoopFinalize before uv_loop_close with draining uv_run passes.
|
Hmmm... Ubuntu 18.04 is using old libuv which does not support |
Use handle->loop via pmfind_handle_loop() so pmfind links on distros whose libuv lacks uv_handle_get_loop (e.g. Ubuntu 18.04).
| * Call before uv_loop_close(); run the loop once to process the close cb. | ||
| */ | ||
| void | ||
| pmWebTimerLoopFinalize(void) |
There was a problem hiding this comment.
Just in terms of naming, "pmWebTimerDestroy" might be more consistent with other parts of libpcp_web?
| uv_run(loop, UV_RUN_DEFAULT); | ||
|
|
||
| uv_close((uv_handle_t *)&timing, NULL); | ||
| uv_run(loop, UV_RUN_DEFAULT); |
There was a problem hiding this comment.
Oh wow, so many explicit calls to uv_run ... are they all really necessary?
|
Thanks @natoscott for the review. The issue itself seems to be orthogonal to the fix delivered in #2555; this patch just revealed it. I will keep working on this and ping you for another review once it is ready. |
2 participants
Add
pmWebTimerLoopFinalize()touv_closethe global instrumentation timer afterpmWebTimerCloseand export it in PCP_WEB_1.24. OrderpmWebGroupClosesostruct webgroupsis freed only fromuv_closecallbacks (timer then async) and clearmodule->privdataearly to avoid double teardown.In
pmfind, defersource_releaseto the next tick to avoid use-after-free whenpmWebGroupDestroyruns fromon_done; delete from contexts dict before destroy; track defer_pending and use extrauv_run/uv_stopas needed. CallpmWebGroupCloseandpmWebTimerLoopFinalizebeforeuv_loop_closewith draininguv_runpasses.This is a followup of the #2555 which fixes unsafe multi-threaded use of timers during request handling.
Later changes fix libuv shutdown rules (do not free memory that still holds closing handles) and pmfind-specific ordering (destroy vs. deref vs. dict, and draining the loop before
uv_loop_close). Those are separate failure modes; fixing one does not imply the other is fixed.