brbzull0 commented on code in PR #13747:
URL: https://github.com/apache/trafficserver/pull/13747#discussion_r4143248328
##########
src/records/unit_tests/test_ConfigReloadTask.cc:
##########
@@ -146,3 +146,24 @@ TEST_CASE("State to string conversion",
"[config][reload][state]")
static_assert(ConfigReloadTask::state_to_string(ConfigReloadTask::State::SUCCESS)
== "success");
static_assert(ConfigReloadTask::state_to_string(ConfigReloadTask::State::FAIL)
== "fail");
}
+
+TEST_CASE("ConfigReloadTask tree is freed with its owner",
"[config][reload][lifetime]")
Review Comment:
suggestion (non-blocking): This test covers the `_parent` cycle. The checker
leak, which is the one #13662 reports, has no test. `test_records` does not
start the event system, so the checker never runs in it. A small test binary
can boot the event system the same way `inkevent_test_fixtures.h` does, then
start ET_TASK with `tasksProcessor.register_event_type()` and
`tasksProcessor.start(1)`. It can then check that the checker releases its task:
```cpp
auto main_task = std::make_shared<ConfigReloadTask>("test-token-checker",
"main task", true, nullptr);
main_task->start_progress_checker();
REQUIRE(main_task.use_count() == 2); // this test + the checker
main_task->mark_as_bad_state("forced");
// First check after check_interval (2 s), confirmation
TERMINAL_CONFIRMATION_DELAY (5 s) later.
auto const deadline = std::chrono::steady_clock::now() +
std::chrono::seconds{15};
while (main_task.use_count() > 1 && std::chrono::steady_clock::now() <
deadline) {
std::this_thread::sleep_for(std::chrono::milliseconds{50});
}
REQUIRE(main_task.use_count() == 1);
```
This fails on master and passes on this branch. It takes about 7 s. To test
the timeout path instead, set `proxy.config.admin.reload.timeout` to `1s` and
skip `mark_as_bad_state()`. That run takes about 2 s, and the record must be
registered first (for example with `LibRecordsConfigInit()`), because
`RecSetRecordString()` fails on an unregistered name.
--
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]
