vy commented on code in PR #4118: URL: https://github.com/apache/logging-log4j2/pull/4118#discussion_r3491136996
########## log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationSchedulerTest.java: ########## Review Comment: The test still passes without the fix, so it does not prove the race is fixed. Also, the current placeholder approach can still leave the returned `CronScheduledFuture` pointing at the already-fired first future if the task runs before `scheduleWithCron()` calls `reset()`. Please make the returned future update monotonic. If the first execution already advanced it to the next scheduled run, the initial `reset()` must not overwrite it with the stale first future. Could implement the following changes, please? - Add a deterministic regression test that fails on current `2.x` - Verify cancellation/stop still cancels the next scheduled cron execution; - Stop the scheduler in a `finally` block in the test; - Add a changelog entry -- 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]
