-
Notifications
You must be signed in to change notification settings - Fork 306
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
FISH-10136 Fix NPE in Payara Micro and Embedded #7065
Conversation
Signed-off-by: Andrew Pielage <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return; | ||
} | ||
|
||
// TODO: Review whether this is still required; Mojarra may have fixed this themselves in PR#5479? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Mojarra fixed this.
It's not applicable to any other Faces implementation,
so I think it's safe to remove it and all it's dependent methods below.
Please double-check with ClassLoaderLeak detector:
https://gist.github.com/lprimak/973ce6289dc1e5ed86dc09b62008de97
Even easier fix: Just change the error log from WARNING to FINE. |
Mojarra handles this itself now, we should no longer need this on our side Signed-off-by: Andrew Pielage <[email protected]>
FISH-10136 Fix NPE in Payara Micro and Embedded
Description
Adds a fallout clause for if field doesn't exist. The field in question belonged in Mojarra, who have since potentially fixed the issue themselves in eclipse-ee4j/mojarra#5479?
Without this change, the following error is thrown on application deployment/undeployment (most prominently in Micro and Embedded)
Need to look into if this entire block of code is still required any more; it seems quite closely tied to Mojarra who have potentially resolved it themselves? Would need to follow it through carefully across redeployments to see if we're still holding on to anything on our side.
Originally introduced here: #6677
Important Info
Blockers
None
Testing
New tests
None
Testing Performed
Started Payara Micro and deployed clusterjsp - no explosions.
Started Payara Server and redeployed classloader-data-api several times - the number of class loaders didn't continually increase.
Testing Environment
Windows 11, Zulu JDK 11.0.25
Documentation
N/A
Notes for Reviewers
None