bryancall opened a new pull request, #13690: URL: https://github.com/apache/trafficserver/pull/13690
One commit, three CIDs, one file. Separated from the rest of #13682 because it is the only change in that series that alters object lifetime in the cache regression machinery, and the only one no ctest case can exercise. ### What leaks `REGRESSION_TEST(RegressionSM)` builds a tree of `RegressionSM` nodes and calls `top_sm->run(pstatus)`. Nothing is ever deleted, which is what CIDs 1022149, 1022150 and 1022151 report. The ownership split matters: - **Composite nodes already self-delete.** The only two terminal exits from a composite's state machine both do it — `RegressionSM::run()` when `nwaiting` hits 0 after the child loop, and `regression_sm_waiting()` when `nwaiting` is 0 on the retry callback. Every other path either reschedules or re-enters `run()`. - **Leaf nodes never did.** `ReRegressionSM::run()` calls `done()` synchronously and returns, so it never schedules and therefore never reaches `regression_sm_waiting()` — which is the only place that would have deleted it. So 12 leaf instances leak per run: the 6 built by the test body, plus 6 more created internally by `RegressionSM::run()`'s repeat path (`children[0]->clone()` for `ichild != n-1`). **Deleting the tree from the test body would be a double-free.** This is worth stating explicitly because it is the obvious-looking fix: a `unique_ptr` vector in the test body (attempted in `0690f09282`, never merged) also cannot reach the internally-created clones. ### Why self-delete is safe here `xrun()` touches no members after `run()` returns; `done()` touches none after `parent->child_done()` returns; the child loop never re-reads the node after `xrun()`; and in the repeat path `children[0]` is only dereferenced for cloning on iterations `0..n-2` and handed over on the last, so it dies last. A parent always outlives its children, since its `nwaiting` only reaches 0 after every child's `child_done()`. This is the same contract `CacheTestSM::complete()` already uses in this subsystem (`done(); delete this;`), so the change makes the two leaf implementations consistent rather than introducing a new rule. ### Verification status — please read before merging `RegressionSM` is a `REGRESSION_TEST` compiled into `inkcache` and invoked via `traffic_server -R`. **No ctest case exercises it**, so this change is compile-verified and reasoned about but has not been executed. It wants an `-R` run. For completeness: CIDs 1021840 and 1021841 (`RegressionTest_cache`, also resource leaks) are **not** addressed here, because nothing in that function leaks. The 12 `CACHE_SM` prototypes are stack objects that are only cloned, never run; the clones are leaves that self-delete via `complete()`; the composites self-delete as above. Those two are recommended for triage as false positives in #13682. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
