FISH-10615 Fix deployment bugs discovered while implementing EAR enhancements #7212
Conversation
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
Pandrex247
changed the title
Contributor
Author
|
|
lprimak
force-pushed
the
fix-deployment-bugs
branch
from
8701480 to
39f8f2e
Compare
Contributor
Author
|
Ready to go. |
lprimak
force-pushed
the
fix-deployment-bugs
branch
from
fba4f2e to
ca72244
Compare
- fixed current deployment context - made it a regular thread local, not queue - only add CDI bundles that are not UNKNOWN - disable injection on EAR-root context (wrong context mode) - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
lprimak
force-pushed
the
fix-deployment-bugs
branch
from
a374a3e to
4f4383c
Compare
Contributor
|
Kindly asking to include this in 6.2025.3! 👍 🙏 |
Member
|
It looks like the previous PRs (#7032 and #7097) actually break the CDI TCK. They are being reverted for now to allow Jakarta certification to proceed. This PR reduces the number of TCK failures from ~30 to 9, but doesn't give us a clean bill of health. |
Contributor
Author
|
@Pandrex247 you're so quick to revert everything :( This PR would have probably fixed all the CDK issues... plus it introduces feature flag |
Contributor
Note that this comes with a cost: It effectively prevents people from upgrading 6.2025.1 to 6.2025.2 due to the severe injection faults reported recently! So unless this PR is merged there should not be another broken 6.2025.3 released, to not confuse users even more with existing-but-not-contained bug fixes. Thanks! BTW, as you are working on that currently, I am about to file another bug report against 6.2025.2: Edit: Documented the new bug in #7233 (comment). |
Pandrex247
changed the base branch from
main
to
FISH-10752-Reapply-EAR-Deployment-Changes-and-Shared-War-Libs
Member
|
I've created a feature branch to aid collaboration and changed the target of this PR to it. CDI TCK results with everything removed (current state of main - 7eab25d): CDI TCK results of the feature branch without this PR (755db21): CDI TCK results of this PR (4f4383c): |
Contributor
Author
|
Have you reapplied all 3 PRs? Can you try the feature flag? to see if that helps? |
Contributor
|
FYI: I assume this new bug (the one in exactly the linked comment, not just the general issue) is related? 🤔 If not, please tell me. Thanks. |
Member
|
With all three applied, and with So no difference, unless I'm applying the system property incorrectly |
Contributor
Author
|
Sorry yes. Set the property to true |
Member
|
No difference with the property set to true |
Contributor
|
Any news? 🤔 |
Member
|
I've had limited time to look at this. The descriptions from the two EnterpriseSelectedAlternative TCK tests seem to indicate things are bleeding across when they shouldn't, regardless of
Meanwhile the ResourceAdapterArchiveTest fails to deploy with a class cast / class loader exception: Another test seems to fail with a similar error, though not at deployment time: ApplicationContextSharedTest @lprimak if you want to have a crack at it yourself, running the CDI TCK is possible locally though it takes a bit of set up:
Running against remote profile is also possible, but requires moving into |
Contributor
Author
Contributor
Author
|
Andrew, can I roll all 3 PRs into one? That would make things a lot easier... |
Member
|
Which three are you referring to? If you're talking about also pulling in #7165 to this PR (or vice versa) - are they related? I just worry you'll end up with a monster PR that'll be hard to review. |
Contributor
Author
Contributor
Fine for me! I really appreciate your dedication! Also, this gives me some time for the missing |
Contributor
|
Don't cry! 🤗 Looks like this really will be the very last bug (at least if we don't break existing fixes, at least for our real-world EAR 😅):
Good luck! 🎯 |
Contributor
Author
|
Got the reproducer. However, I am not sure I understand what the desired behavior is, as it looks like the reproducer is working properly. |
Contributor
Author
|
|
Contributor
|
I only need to execute That exception of yours is what everybody expects to actually get: It is what should happen without the bug! So it proofs that the bug exists only on Windows. I assume that on Windows (and only on Windows) some special thread group is used. I can hide the bug (and get your correct stack trace) as soon as I don't spawn a process, but simply use my own thread group. Also I get your correct stack trace as soon as the binary (here: So how to proceed? 🤔 I can offer to install remote access software on my dev VM so you can remote debug on Windows (you did that some years back in a related case). |
Contributor
|
Anyways, you are right. I have deploy the reproducer to Payara CE 6.2025.4 and the bug shows up there, too. So the bug is not related to your PR. Hence, the bug does not stand in the way of merging it. 😃 I will report it against 6.2025.4 in a separate issue. |
Contributor
|
Reported against Payara CE 6.2025.4: #7324 |
Contributor
|
@Pandrex247 I hereby confirm that this PR has no more open issues with our real-world EAR. Hence I do not see any need to further delay merging it to trunk. Having said that, as code was changed since the last TCK run, it might make sense to first run the TCK and all other CDI-related tests before merging. |
Member
|
I kicked off a full run last night. I'm verifying if these are valid or if Jenkins just had a blip. |
Contributor
This is weird. I cannot see how full server can succeed while web-only fails?! 🤔 |
pzygielo
reviewed
Apr 30, 2025
appserver/web/weld-integration/src/main/java/org/glassfish/weld/ACLSingletonProvider.java
Outdated
Show resolved
Hide resolved
Contributor
Author
@Pandrex247 Yes, this is weird. Do you have a sample stack trace I could look at? I'd just like to sanity check if this is at all related, or could be related. Thank you |
Member
|
They seem to be failing consistently. |
Member
|
So... The CDI TCK fails on So the only thing I can give you is the source and the line it fails on really: ExcludeFiltersTest For WebSockets, I'm really not sure what goes wrong - the web profile distribution of the server just seems to immediately zombie. The TCK runner starts off by deleting domain1 and recreating it with some different ports (which seems to work fine) before then attempting to start the domain - this start just hangs almost immediately. As far as I can tell from the runner it hasn't copied any extra libraries in or done any other config yet - it just immediately hangs on startup. I can actually verify this locally too - any attempt to start even a clean payara-web has it immediately hang. |
Contributor
Author
|
Ok, I'm able to reproduce the initial hang. This is the root cause of the web TCK failure. It's really weird. Must be some other bug that was uncovered. This is not related to my changes... stay tuned |
Contributor
Author
|
Well, I take it back. The issue is with the code I touched :) |
Contributor
Author
|
@Pandrex247 Please run the TCK on the other branch (which is up-to-date with main) |
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 1, 2025
…I-enabled library JARs (payara#7212) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 1, 2025
…I-enabled library JARs (payara#7032) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found (payara#7212) - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 5, 2025
…I-enabled library JARs (payara#7032) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found (payara#7212) - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
…n-ear-libs` system proeprty that's no longer necessary
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 5, 2025
…I-enabled library JARs (payara#7032) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found (payara#7212) - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 5, 2025
…I-enabled library JARs (payara#7032) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found (payara#7212) - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
Contributor
Author
|
Superseded by #7314 |
lprimak
added a commit
to flowlogix/Payara
that referenced
this pull request
May 6, 2025
…I-enabled library JARs (payara#7032) - correctly copy BDA sets for each war in EAR - Make WAR's CDI beans available in EAR-libs - read web-fragment.xml from EAR-libs - processing ear-lib manifest - de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList - made some structures final (cleanup) - fixed ear and concurrent classloader leaks, including refactored reflection caching enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097) bugfix: EAR deployment bugs found (payara#7212) - fixed current deployment context - made it a regular thread local, not queue - actually destroy deployment ClassLoader shareableTemp correctly - added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment - Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
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.
Description
While working on EAR enhancements, I ran into a few bugs. These are fixed by this PR:
leading to deployment failures on artifacts that should succeed.
nullkey discovered inbundleToBeanDeploymentArchive.put()methodcurrentDeploymentContextfromThreadLocal<Deque>toThreadLocalasDequewas unnecessary and causing bugsfish.payara.ear-combined-cdi-deploymentApplication and System property to force "old" EAR behaviorCommit message:
Fixes #7201
Fixes #7214
Fixes #7215
Fixes #7233