Fix NPE in ConfigurationScheduler by ensuring scheduledFuture is chec…#4118
Open
ramanathan1504 wants to merge 29 commits into
Open
Fix NPE in ConfigurationScheduler by ensuring scheduledFuture is chec…#4118ramanathan1504 wants to merge 29 commits into
ramanathan1504 wants to merge 29 commits into
Conversation
…ked for null before accessing its methods; add test for race condition in scheduling.
vy
requested changes
May 28, 2026
vy
left a comment
vy
left a comment
Member
There was a problem hiding this comment.
@ramanathan1504, please add a changelog entry too.
Member
There was a problem hiding this comment.
@ramanathan1504, test passes without your changes. Could you make sure this test fails without the fix?
Contributor
Author
There was a problem hiding this comment.
@vy , I restored the file log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationScheduler.java. After doing so, the build succeeded—the test case passes perfectly without requiring any modifications to the source file.
Member
There was a problem hiding this 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
finallyblock in the test; - Add a changelog entry
github-project-automation
Bot
moved this to Changes requested
in Log4j pull request tracker
vy
requested changes
Jun 29, 2026
Member
There was a problem hiding this 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
finallyblock in the test; - Add a changelog entry
apache#4120) * Restrict JUnit dependencies to below version 6.x to maintain compatibility with Java 8 * Update JUnit dependency restrictions for compatibility with Java 8 on 2.x
Remove the version override for `apache-rat-plugin` so it falls back to the version provided by the ASF Parent POM. Apache RAT `0.18`, for which Dependabot opened an update PR, contains a bug ([RAT-552](https://issues.apache.org/jira/browse/RAT-552)) that effectively disables the `excludeSubProjects` plugin option whenever `<excludes>` is present in the configuration. Upgrading to `0.18` will require converting our `<excludes>` to `<inputExcludes>`, which depends on a new `logging-parent` release.
…oss 1 directory (apache#4121) * Bump actions/stale in the dependencies group across 1 directory Bumps the dependencies group with 1 update in the / directory: [actions/stale](https://github.com/actions/stale). Updates `actions/stale` from 9.1.0 to 10.2.0 - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@5bef64f...b5d41d4) --- updated-dependencies: - dependency-name: actions/stale dependency-version: 10.2.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: dependencies ... Signed-off-by: dependabot[bot] <support@github.com> * Generate changelog entries for apache#4121 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Fix KafkaAppender reporting error after successful retry When retryCount is configured and the initial tryAppend() fails, the retry loop uses break to exit on success. However, break only exits the while loop and execution always reaches the error() call afterward. This causes spurious error notifications for transient Kafka failures that were successfully recovered by a retry. Replace break with return so that a successful retry exits append() without reporting an error. Retry exceptions are now logged at DEBUG level for diagnostics instead of being silently discarded. Also remove dead code in Builder.getRetryCount() where Integer.valueOf(int) was wrapped in a NumberFormatException catch that can never fire. The bug was introduced in apache#315. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * Add changelog entry for KafkaAppender retry fix Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * Use CloseableThreadContext in testRetrySuccessDoesNotReportError Adopt reviewer suggestion to use try-with-resources CloseableThreadContext instead of manual ThreadContext.put/clearMap. --------- Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Co-authored-by: Björn Michael <b.michael@gmx.de> Co-authored-by: Volkan Yazıcı <volkan.yazici@oracle.com>
…g logic (apache#4081) Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
…4071) Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
…ptor library (apache#4124) Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
…pache#4133) * Fix circular reference detection for exceptions with colliding equals/hashCode implementations * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add changelog entry for circular reference detection fix with colliding equals/hashCode exceptions * Add test for ThrowableProxy serialization with colliding equals/hashCode implementations * Refactor ThrowableProxy serialization test to use modern Java I/O classes * Use `TestFriendlyException` to exercise the malfunction * Update changelog * Improve comments * Remove redundant change --------- Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
Co-authored-by: Jeff Thomas <jeffrey.thomas@t-systems.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
…#4098) Signed-off-by: SunWeb3Sec <infosecpt@gmail.com> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
…che#4127) Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> Co-authored-by: Ramanathan <ramanathanbscmca@gmail.com> Co-authored-by: Piotr P. Karwasz <pkarwasz-github@apache.org> Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
Applies the consumer-side changes from logging-parent PR apache#476: * `build.yaml` and `codeql-analysis.yaml` now subscribe to the `ready_for_review` pull request type, so required checks re-run when a Dependabot PR is taken out of draft. * `process-dependabot.yaml` drops the `RECURSIVE_TOKEN` PAT secret, which is no longer needed now that the reusable workflow parks the PR in draft mode instead of pushing with a privileged token. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Configure a Maven update entry targeting the `2.26.x` branch that only proposes patch-level upgrades. All dependencies are bundled into a single `Maven patch updates` group with a 7-day cooldown.
Instead of correcting the target branch, we can safely remove it: the only dependency version defined in `log4j-slf4j-impl` is SLF4J 1.x, which is highly unlikely to see any new releases.
* Rework random delay for Rolling Appender action chain * Stabilize `RollingAppenderDirectCronTest`
…apache#4171) * Add tracing fields to RingBufferLogEvent and related classes * Add W3C trace context support with pattern converters for trace ID, span ID, and trace flags * Enhance TraceContextProviderService for exception safety and simplify trace ID retrieval * Add tests for tracing fields serialization and pattern converters * Refactor tracing metadata documentation and improve code comments for clarity * Add native W3C tracing fields to LogEvent and introduce TraceContextProvider SPI * Add benchmark for ContextDataProvider tracing approach * Enhance TraceContextProviderService to handle SecurityManager restrictions gracefully
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2073 existing Pr #3941
Fixed the NPE issue and added a test case for
ConfigurationScheduler