Skip to content

bugfix: clean up effectiveInjectionManager and move cleanup into fina…#6099

Open
lprimak wants to merge 1 commit into
eclipse-ee4j:4.xfrom
lprimak:fix-threadlocal-leak
Open

bugfix: clean up effectiveInjectionManager and move cleanup into fina…#6099
lprimak wants to merge 1 commit into
eclipse-ee4j:4.xfrom
lprimak:fix-threadlocal-leak

Conversation

@lprimak

@lprimak lprimak commented Jul 2, 2026

Copy link
Copy Markdown

…lly block

follow-on to #6049 regression

@lprimak

lprimak commented Jul 2, 2026

Copy link
Copy Markdown
Author

@OndroMih See this PR

@OndroMih

OndroMih commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

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

@lprimak

lprimak commented Jul 2, 2026

Copy link
Copy Markdown
Author

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

lprimak commented Jul 2, 2026

Copy link
Copy Markdown
Author

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

lprimak commented Jul 3, 2026

Copy link
Copy Markdown
Author

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

@OndroMih

OndroMih commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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

OndroMih commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@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

lprimak commented Jul 3, 2026

Copy link
Copy Markdown
Author

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

OndroMih commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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

OndroMih commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@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

lprimak commented Jul 4, 2026

Copy link
Copy Markdown
Author

@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.

Untitled

@lprimak

lprimak commented Jul 4, 2026

Copy link
Copy Markdown
Author

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

lprimak commented Jul 4, 2026

Copy link
Copy Markdown
Author

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

@OndroMih

OndroMih commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

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:

image

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:

image

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
image

@OndroMih

OndroMih commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

@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

lprimak commented Jul 5, 2026

Copy link
Copy Markdown
Author

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

@lprimak

lprimak commented Jul 5, 2026

Copy link
Copy Markdown
Author

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

lprimak commented Jul 5, 2026

Copy link
Copy Markdown
Author

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

@lprimak

lprimak commented Jul 5, 2026

Copy link
Copy Markdown
Author

@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

OndroMih commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

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

lprimak commented Jul 5, 2026

Copy link
Copy Markdown
Author

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

@lprimak
lprimak force-pushed the fix-threadlocal-leak branch 2 times, most recently from 739f880 to 5cc0461 Compare July 12, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants