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
TOMEE-4192 ApplicationComposers do not clear GC references on release #1030
Conversation
- Added Inherited annotation. - Added unittests to proove fix.
Thank you all... |
Thanks for the PR @G-Ork ! |
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.
Looks like we have some regressions with this change:
org.apache.openejb.junit.ContainerAndApplicationRulesTest.test
org.apache.openejb.junit.ContainerApplicationRuleTest.run
specifically:
java.lang.NullPointerException
at org.apache.openejb.testing.ApplicationComposers.after(ApplicationComposers.java:1112)
at org.apache.openejb.junit.ContainerRule$1.evaluate(ContainerRule.java:47)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:377)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:284)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:248)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:167)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581
Would you mind having a look? I guess, that it would be sufficient to add null
checks.
CI:
Hi I will look. Give me some days to overcome corona.
Richard Zowalla ***@***.***> schrieb am Mo., 27. März 2023,
12:56:
… ***@***.**** requested changes on this pull request.
Looks like we have some regressions with this change:
org.apache.openejb.junit.ContainerAndApplicationRulesTest.test
org.apache.openejb.junit.ContainerApplicationRuleTest.run
specifically:
java.lang.NullPointerException
at org.apache.openejb.testing.ApplicationComposers.after(ApplicationComposers.java:1112)
at org.apache.openejb.junit.ContainerRule$1.evaluate(ContainerRule.java:47)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:377)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:284)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:248)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:167)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581
Would you mind having a look?
—
Reply to this email directly, view it on GitHub
<#1030 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGZCNF6D3SIZNBW3BRWZ5QDW6FW7FANCNFSM6AAAAAAWACYBRY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sure. Get well soon (also had it last week, therefore my late response) |
case of legacy junit rules.
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've changed the code to integrate better with older junit testrules:
- Added an null-check to prevent NPE
- move the reference resets to stopApplication()
this should also free references in older junit tests. The Fix is necessary because in this case two composers are active. One do not have an appContext initialized. The one with an context do not call after() just stopApplication().
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.
Builds (9,8) looking good with this change. Thanks for the PR @G-Ork
Thanks @G-Ork |
…apache#1030) * TOMEE-4192 ApplicationComposers do not clear GC references on release
No description provided.