bugfix: clean up effectiveInjectionManager and move cleanup into fina…

[lprimak]

…lly block

follow-on to #6049 regression

[lprimak]

@OndroMih See this PR

[OndroMih]

How does this fix the leak? Which thread local is the cause of the leak, effectiveInjectionManager or threadInjectionManagers?

[lprimak]

effectiveInjectionManager is causing the leak. setInjectionManager() sets it, but it never gets cleared.
I put threadInjectionManagers in the finally block because it's the correct thing to do, and correct pattern to do so, in case there is an exception somewhere in the try block and no new leak is caused.

[lprimak]

Looks like CI passed. There seem to be tests that check integration against weld and HK2.
Do you think this means that the code will work properly? In other words, do you trust the tests would actually break if this PR did not work properly?

[lprimak]

I tested this PR in Payara, and it works. Rest APIs work, there are no errors, and there are no longer ClassLoader leaks.

[OndroMih]

I don't know enough about Jersey code base to tell you right now that all is OK with this change. I'm afraid that it might break some cases that are not covered by tests because the scenario I'm concerned about is sort of a special case - inject would need to be called twice. I'll see if there's a test that covers that scenario and will try to write a test case for it but I don't know when I will have time for that.

If you want to help me, try creating a test or modify your reproducer so that inject is called multiple times, if that's possible. I would try with creating some CDI beans that contain several @Inject fields.

[OndroMih]

@lprimak , do you see "Leaking injection manager found in the current thread" errors printed from the setInjectionManager method? If effectiveInjectionManager leaks, then some later invocation on the same thread should detect it and log this error. A partial solution would be to reset the effectiveInjectionManager to the injectionManager in that case. It wouldn't fix the leak but at least provide the correct injectionManager to the application.

[lprimak]

Ondro, I have tested this with a real application that has 100s of Injected fields, it works fine.

Have you by any chance seen the initial issue that was filed against Payara for this?
payara/Payara#8260
Yes, the error is printed out.

I really have done all I can here, going far "above and beyond" since I am not working for Payara, or OmniFish, etc.

[OndroMih]

Hello, @lprimak

I ran the reproducer https://github.com/AndrewG10i/jakartaFacesTestApp/ according to your instructions, and I see the following:

• running with Payara Micro 7.2026.6 -> I see the errors about the leak in Jersey
• running with Embedded GlassFish 8.0.3 -> I see no errors
• running with a modified Payara Micro 7.2026.6 with stock Jersey 4.0.2 jars dropped into MICRO-INF/runtime, replacing the Jersey jars bundled in Payara Micro -> I see no errors

So it seems to me that the issue is only in Payara Micro and only in the patched Jersey version in Payara Micro. Please consult with the Payara team or patch your Payara Micro version with stock Jersey jars.

[OndroMih]

@lprimak , please confirm that the issue is gone if you replace Jersey jars in Payara Micro with the stock Jersey 4.0.2 jars (jersey-cdi1x, jersey-cdi1x-servlet, and jersey-server were enough for me to get rid of the issue, maybe even jersey-server is enough).

If that's the case, please ask the Payara team for support. I don't think we can accept a fix for something that is not reproducible with the upstream Jersey artifacts.

[lprimak]

@OndroMih ,
The issue does not always result in the errors being printed on the console. The errors are only the symptom, not the cause. Absence of the errors does not indicate that the issue is not present.

As suggested, I ran Payara with stock (un-patched) jersey-cdi1x, jersey-cdi1x-servlet, and jersey-server
The issue is still there, evidenced by:

• ClassLoader leaks
• Debugging the behavior, and confirming that the ThreadLocals are not removed after setting them

I also ran GlassFish 8.0.3. However, because GlassFIsh has many more ClassLoader leaks, I cannot pinpoint which is causing them.

[lprimak]

I ran the debugger with unpatched Jersey 4.0.2
Put breakpoints:

• CdiComponentProvider:725 (set)
• CdiComponentProvider:271 (remove)

Watch that set is called multiple times, but remove() is not called after each set()

Further elaborating, the initialize() flow is correctly cleared via postInit() flow.
However, subsequent AbstractCdiBeanSupplier._provide() / getInstance() flow is not properly cleared at all.

[lprimak]

Also, this leads to weird subsequent deployment and hanging issues, since getInstance() sometimes uses stale thread-local instances.

[OndroMih]

It's possible that GlassFish 8.0.3 has some other leaks but I don't see this specific leak. None of the WebAppClassloader instances in GlassFish after I deploy and undeploy the app is referenced from a ThreadLocal of type ImmediateHk2InjectionManager as in Payara. What I see in GlassFish is several WebAppClassloader instances related to the Admin UI and 1 related to the undeployed app. But that is referenced from a JarFileManager thread, which inherits it via thread context CL from the parent thread. I scanned all thread locals in all threads via a debugger and I didn't find any thread local that would recursively reference a WebAppClassloader instance in GlassFish or Payara with the stock Jersey jar.

I tried again replacing Payara's jersey-server.jar in Payara 7.2026.6 with the stock jersey-server 4.0.2 and I stopped seeing the leak. After deployment and undeployment, there's a single WebAppClassloader instance, for the Admin UI, while before I saw what you shared - 2 instances, one of them held by a ImmediateHk2InjectionManager threadLocal.

Did you delete osgi-cache after you replaced jersey-server.jar, @lprimak ?

Here's what I see in raw Payara 7.2026.6 - 2 instances of WebAppClassloader, one is expected (admin UI) and the other one is held by the thread local:

After I replaced the jersey-server.jar, deleted osgi-cache from the domain directory and repeated the reproducer, I only saw a single instance for the admin console, no other WebAppClassloader instance remaining after undeployment:

In GlassFish 8.0.3, I also don't see WebAppClassloader leaking from the ImmediateHk2InjectionManager thread local, I just see 5 WebAppClassloader instances either leaking via inheritableThreadLocals or held by some other GlassFish components which should probably have released the references:

• 2 references are held by ContainerBackgroundProcessor threads
• 1 is held by an OSGi classloader
• 1 by weld-worker-6 thread
• 1 is in inheritableThreadLocals map of a ForkJoinPool thread - this is the only one that is related to the undeployed app, all others are related to the Admin UI. This is a leak caused by a ForkJoinPool thread inheriting a thread local of type InvocationManagerImpl$InvocationArray, not related to Jersey in any way

[OndroMih]

@lprimak , please, try again with stock Jersey JARs in Payara and delete osgi-cache. If you still see leaks, please create a script or a Dockerfile which demonstrates what you're doing. I really can reproduce your issue only on the default Payara setup but not with Payara and stock Jersey 4.0.2 or GlassFish 8.0.3 (with the same Jersey 4.0.2).

[lprimak]

I not only tried deleting osgi-cache but started with fresh domain.

[lprimak]

Also did you actually use the reproducer app? It's not enough just to deploy and undeploy it, but you need to use the deployment.

Best way is the debugger like I mentioned above.

[lprimak]

Also, you are not going to see this specific leak in GlassFish, because other leaks are hiding this particular leak.

[lprimak]

@OndroMih ,

I have created a brain-dead-simple reproducer for you:
https://github.com/lprimak/gf-jersey-leak-repro
It has all the instructions in the README.
This uses stock GlassFish 8.0.3 and demonstrates the problem in the debugger.
No Payara, no patching. just requires a checkout of Jersey with 4.0.2 tag and a debugger.

You will see that there is no remove() after the last set() call, which demonstrates the bug.
You will not be able to see any logs.

[OndroMih]

I didn't see this reproducer linked anywhere. I was using https://github.com/AndrewG10i/jakartaFacesTestApp.git which I believe you linked somewhere in one of the issues.

Next time, please raise a proper issue on the Jersey or GlassFish project instead of distributing information across multiple issues and PRs. I'm getting lost.

Your reproducer also doesn't describe the scenario that leads to the leak. Or do the integration tests trigger the leak? Please, describe what I should do to reproduce it if plain deploy and undeploy isn't enough. Ideally the minimum scenario to reproduce it.

[lprimak]

I am trying my best here.
The README describes exact reproducing steps. Yes the integration test triggers the issue.

[mkarg]

Using https://github.com/lprimak/gf-jersey-leak-repro I was able to reproduce the described bug (thank you @lprimak for setting it up, it was really simple to follow your instructions in README.md and trace using jdb -attach localhost:9009. Indeed, it proofs that Jersey (in this case included in a Payara-free environment based on GlassFish 8) invokes effectiveInjectionManager.set() multiple times while ./mvnw verify -Pserver-local is running, but effectiveInjectionManager.remove() only once. IMHO this perfectly proofs the cause and is rather simple to understand to reproduce.

[mkarg]

Looking at the original (unmodified) code, it says it deliberately wants to share instances, and likes to do effectiveInjectionManager.remove() only in done(). I understand that your solution works, @lprimak , as it performes .remove() directly after each injection, but IMHO this spoils the idea of reusing the reference multiple times.

So I wonder if the proposed solution is actually tackling the root cause of the behavior? IMHO I see two possible causes:

• Either done() is not called between multiple invocations of inject() by mistake, then we should fix that. 🤔
• Or it is simply pretty valid that effectiveInjectionManager.remove() is not called, so the error message Leaking injection manager found in the current thread makes simply no sense as that behavior is expected. 🤔

As it was @OndroMih who added that message I would kindly ask him to elaborate about that question.

Nevertheless, I wonder if there is any harm other than printing Leaking injection manager found in the current thread in the log, actually? 🤔

[OndroMih]

My previous fix was aimed for the client side and the assumption was that done() is always called at the end. If a different injectionManager is detected, it’s unexpected and means there’s a leak.

Though I don’t know how it should work on the server side. I would assume that done() is called at least after esch request and then the thread local would be removed from the thread. If done() isn’t called on purpose then we should find why and introduce a different mechanism that would remove the thread local when the thread is done with processing.

But even on server side, I expect that having a different injection manager from the one that is being set, is not expected and it comes from a leak. I would definitely like to have a solution that runs the done() method after each request. The situation might get a bit tricky with asynchronous processes (suspended requests), in that case the thread local should be cleared also for the asynchronous thread.

[lprimak]

Guys,

Since I am not familiar with the codebase, I had AI analyze the code paths.
I don't really trust AI right off the bat, so I double-checked each of the AI assumptions with the debugger in the code.

done() is called from initialization code path.
There is another code path (AbstractCdiBeanSupplier -> CdiComponentProvider.InjectionManagerInjectedCdiTarget) where the thread-local is not cleaned up.

The reuse just simply never happens.

What the AI is saying is what I confirmed to be correct in debugging sessions.
Furthermore, all Payara's tests pass with my fix. I cannot try the TCK.

I am not sure what else I can do to increase confidence in the fix.
All I know is the current state is completely broken.

Q:

can managedTarget.inject() be called multiple times?

A:

In AbstractCdiBeanSupplier, hk2managedTarget is created fresh inside getInstance(), then used like this:
setInjectionManager(...)
inject(instance, creationalContext)
postConstruct(instance)
So in the current implementation it is meant to be called once per produced instance.
A few details from CdiComponentProvider.InjectionManagerInjectedCdiTarget:
There is no explicit guard preventing a second inject() call.
But the code and comments assume single-invocation semantics:
setInjectionManager() says the manager is “always the same within a single invocation”
the method clears thread-locals in finally, which fits one-shot use
Calling inject() again on the same target/instance would likely repeat member injection and could lead to duplicate or inconsistent lifecycle behavior.

So the practical answer is:
Technically: yes, nothing in the class forbids another call.
By design: it should be treated as one-time per instance, not something to call multiple times.
If you want, I can also trace whether CDI’s InjectionTarget.inject() contract allows reinvocation or whether this wrapper relies on a stricter assumption.

Q:

I just changed inject() to clear effectiveInjectionManager in finally block. maybe that's the wong thing to do? did I just change the assumption once-per-call?

A:

Yes — clearing effectiveInjectionManager in the finally block is the right thing, and it does not change the intended once-per-call assumption.
Checklist
Confirm whether the finally cleanup is correct
Clarify what “once-per-call” means here
Explain the effect on repeated inject() calls
What the code is assuming
InjectionManagerInjectedCdiTarget is using thread-local state to support one self-contained injection invocation:
setInjectionManager(...)
inject(...)
postConstruct(...)
cleanup in finally
That cleanup is there to prevent the InjectionManager from leaking into the next invocation on the same thread.
So did you change the assumption?
No — you enforced it.
By clearing effectiveInjectionManager in finally, you’re saying:
this invocation is complete now
any later call must set up its own manager again
the target should not rely on leftover thread-local state
That is consistent with the existing comment:
“The injectionManager is always the same within a single invocation.”
Important nuance
This does not mean the InjectionManagerInjectedCdiTarget object itself can never be reused.
It means each call sequence must be independent. If someone wants to call inject() again later, they must first call setInjectionManager(...) again for that new invocation.
Why cleanup matters
Without removing the thread-local state:
a later call on the same thread could accidentally reuse a stale manager
Hk2Bean.create() and nested CDI/HK2 bridging could see the wrong context
you’d risk thread-local leakage across requests or reloads
Bottom line
Clearing in finally is correct
It does not introduce the once-per-call assumption
It actually protects that assumption from leaking across invocations
If you want, I can also walk through the exact call flow from AbstractCdiBeanSupplier to show why the cleanup has to happen after delegate.inject(...) and before the method returns.

[OndroMih]

Looking more into the code, I see one place that proves what your AI claims: in internal/AbstractCdiBeanSupplier.java, setInjectionManager() is followed by a single call to inject() and then a single call to postConstruct(). If that’s the only way how inject is invoked, always after setInjectionManager and always exacrly once, then your PR is OK. I would just like to have more proof that it’s the only valid way how inject is called everywhere in the code.

Maybe it would still be better to move removing rhe effectiveInjectionManager value in the postConstruct method, it seems more logical to me. It wouldn’t change anything but it would at least make more logical sense. It’s now removed in the done() method, which is used during Jersey initialization, calling it in postConstruct during CDI lifecycle makes more sense than calling it in inject.

[lprimak]

Disclaimer: I have not checked this in the debugger...

That would be too early and likely break the flow.
Checklist
Compare inject() vs postConstruct() responsibilities
Check how effectiveInjectionManager is consumed later
Explain why cleanup belongs in finally, not postConstruct
Why moving it to CDI postConstruct is risky
In InjectionManagerInjectedCdiTarget.inject(...), effectiveInjectionManager is used to bridge CDI/HK2 during:
delegate.inject(t, cc)
injectingManager.inject(t, CdiComponentProvider.CDI_CLASS_ANALYZER)
Only after both of those complete does the code clear the thread-local in finally.
If you moved effectiveInjectionManager.remove() into postConstruct(...):
postConstruct would run after inject() in the normal flow, so it might seem equivalent at first
but inject() itself still needs the thread-local to remain available for:
nested HK2/CDI bridging
Hk2Bean.create() calls that can happen during injection
any downstream code that depends on getEffectiveInjectionManager()
So the cleanup must happen after the entire inject sequence, not inside postConstruct.
The important detail
postConstruct() is not the cleanup boundary for this bridge. The real boundary is:
the full injection invocation ends
then thread-local state is removed
That’s why finally is the correct place:
it runs whether injection succeeds or fails
it prevents leakage across invocations
it preserves the manager for the whole injection lifecycle
If you remove it in postConstruct
You risk breaking cases where:
CDI/HK2 still needs the manager after member injection but before lifecycle completion
nested calls happen during postConstruct-adjacent processing
error handling or retry logic depends on the manager still being present
Bottom line
Don’t move effectiveInjectionManager.remove() into postConstruct().
Keep it in finally inside inject() so the full injection lifecycle has access to the manager and the thread-local is cleared only when that invocation is truly done.
If you want, I can draw the exact call sequence between AbstractCdiBeanSupplier, InjectionManagerInjectedCdiTarget, and Hk2Bean to show where the thread-local is required.

[OndroMih]

Maybe even better is to introduce a nee finalization method and do the cleanup there, and call that finalization method in a finally block, even if inject or postConstruct throw an exception. If it’s possible I mean.

[lprimak]

Dunno. Feel free to update my PR :) I am ok with any improvements, as long as they don't break things or cause a class loader leak :)

[OndroMih]

When I get to it I’ll try what I suggested.

By the way, your AI assistant contradicts itself - postConstruct is called after inject but your AI claims that if the cleanup happens in postConstruct it would be too early and it should rather be cleaned up in the inject method. It didn’t earn my trust, really.

[lprimak]

Mine either :) I only trusted what I saw in the debugger and passing tests :)

[mkarg]

IIUC @lprimak's PR is working correctly, and we only discuss if we could provide a more logical solution? As this might take considerable time, it might be good to first merge his PR ASAP (so we can deliver a fix in 4.0.3 to users ASAP) and search for a better software design unhurriedly at a later time? WDYT? 🤔 Or did I misunderstand the thread so far and we are not sure if this PR won't break something?

[OndroMih]

Yes, the reason for the discussion is that I’m not sure whether it can break something. It looks like it can’t break anything in the current code base but I’m still not 100% sure.

We can merge it though, it probably fixes more than it can break. And if it breaks something, it would be some rarely used usecase.

I’m not a committer, so I can’t approve.

[mkarg]

I think it helps more than it potentially breaks, so +1 for merging. @dmatej @arjantijms WDYT?

[lprimak]

Agree. This PR fixes clearly broken logic. Any possible regressions are just guesses at the moment.

Also any possible improvements are risks for more regressions

I agree with Markus. Merge this first and perfect the logic later.

[mkarg]

@eclipse-ee4j/ee4j-jersey-committers Kindly asking for your votes. If nobody vetos, I will merge later today. Thank you.