Skip to content

fix: correctly detect Disruptor major version #3778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 28, 2025
Merged

Conversation

ppkarwasz
Copy link
Contributor

This PR updates the Disruptor version detection logic to use the classloader that loaded DisruptorUtil, instead of relying on the thread-context classloader. The previous approach used LoaderUtil.isClassAvailable(), which can fail in environments where Disruptor classes are not visible to the thread-context classloader.

Context

Long-term, we plan to remove or significantly limit the use of the thread-context classloader (see #2850). However, addressing that comprehensively requires auditing all usages of the Loader and LoaderUtil classes, which introduces risk if any call sites are overlooked.

To reduce disruption, this PR introduces a minimal, targeted fix for this specific issue in version 2.25.1. The broader change tracked in #2850 will be addressed in version 2.26.0.

Ensure that DisruptorTest explicitly fails when an exception occurs on an asynchronous thread. This improves error detection and prevents silent test passes in the presence of async failures.
Ensure the Disruptor version is detected using the classloader that loaded `DisruptorUtil`, rather than the thread-context classloader. The previous implementation relied on `LoaderUtil.isClassAvailable`, which may fail in environments where the Disruptor classes aren't visible to the thread-context classloader.
@vy vy added bug Incorrect, unexpected, or unintended behavior of existing code dependencies Related to third party dependency updates or migrations async Affects asynchronous loggers or appenders labels Jun 27, 2025
@vy vy added this to the 2.25.1 milestone Jun 27, 2025
@ppkarwasz ppkarwasz merged commit 32d29ae into 2.x Jun 28, 2025
11 checks passed
@ppkarwasz ppkarwasz deleted the fix/3706_disruptor-tccl branch June 28, 2025 07:14
@github-project-automation github-project-automation bot moved this from To triage to Done in Log4j bug tracker Jun 28, 2025
@ppkarwasz ppkarwasz removed this from the 2.25.1 milestone Jul 1, 2025
ppkarwasz added a commit that referenced this pull request Jul 5, 2025
### fix(test): fail `DisruptorTest` on async thread exceptions

Ensure that DisruptorTest explicitly fails when an exception occurs on an asynchronous thread. This improves error detection and prevents silent test passes in the presence of async failures.

### fix: correctly detect Disruptor major version

Ensure the Disruptor version is detected using the classloader that loaded `DisruptorUtil`, rather than the thread-context classloader. The previous implementation relied on `LoaderUtil.isClassAvailable`, which may fail in environments where the Disruptor classes aren't visible to the thread-context classloader.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async Affects asynchronous loggers or appenders bug Incorrect, unexpected, or unintended behavior of existing code dependencies Related to third party dependency updates or migrations
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

NPE in org.apache.logging.log4j.core.async.RingBufferLogEventHandler4.notifyCallback()
2 participants