-
Notifications
You must be signed in to change notification settings - Fork 722
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
jdk22 WrapperHiddenClassTest::testModule segfault in libj9gc #18805
Comments
We observed this kind of problem before. Looks like NULL is sent instead of the string. Is there missed NULL check? Or something is NULL unexpectedly? |
At the point of the crash, the object does not seem to be NULL.
|
I added comp:vm label to keep this issue on VM radar. |
Looking to original core:
|
BTW many DDR modules related commands don't work with this core, for example |
There are two |
@dmitripivkine As requested, the zip contains a system core, java core and JDK22 created on Linux x64: https://ibm.box.com/shared/static/f8e1083e2laz5rwp9r33nua52fx0atv3.zip. |
@babsingh Thank you for core/jdk, it was very useful! I have traced failure back to
|
@dmitripivkine I discussed the issue with @JasonFengJ9 and @hangshao0. Here is the summary: With the re-implementation of
|
Freeing the native But, So, we will need a more custom callback for the GC, where the |
I don't think this is a good idea to add such code at GC side. Instead why not to add this code at VM side to the function subscribed to |
Is |
FYI: Currently during class unloading we are triggering hooks:
|
No. Hidden/anon classes can be independently unloaded; it doesn't depend on the associated class loader to be cleared/unloaded.
We can't link this step with class unloading because multiple hidden classes can be associated to the module. VM won't be able to determine the liveness of the module. For the VM to handle it, we will need a new hook which will be triggered once the module is cleared by the GC. |
The question was which class loader is referenced from module for Anonymous class: special Anonymous Classloader or Anonymous class logical parent class loader ? |
What is a liveness condition for the module? |
In JDK22, only hidden classes are used; anon classes are not used. for a hidden class, the class loader is defined by the Java API; it can either be the lookup class's classloader or it can be the bootstrap classloader. MethodHandles.ClassDefiner has more details.
When the GC clears the |
As we see GC explicitly keeps java.lang.Module objects alive by searching them in
|
I dont think there is anything special about this particular use case. A new module is created for the hidden class. The class will disappear when it is no longer referenced, but the module should remain alive until the classloader dies. Ive verified that the j9module in question is in the At the start of the first GC the module->moduleName is valid (Note: the name is tenured). However, on every susbsequent GC, module->moduleName changes. At the time of the crash it is pointing to a weak reference. The original String that module->moduleName pointed to at the time of the first GC is still valid, even at the time of the crash. AFAIK moduleName is never written to after module creation. So this would impl some kind of corruption. |
My observations are identical to @tajila's observations in #18805 (comment). J9Module's life cycle:
The failure happens only when the below code is run, WrapperHiddenClassTest.java#L208-L210:
|
[UPDATE] Tried the GC options provided by @dmitripivkine. The test passes with Core files with |
Looking to the latest core (with optthruput) am not sure it is related, probably not. However:
this class was Anonymous (or Hidden) obviously. And BTW this time bad pointer from corrupted
|
I monitored the object in question on every GC. The object seems to be scanned in every GC. The only place where I see the field being changed is below.
On one of the GCs after I noticed that |
At this point I think the issue is in the GC. The VM doesnt touch the field at all. And the module is scanned every GC. |
@dmitripivkine should the old and new value be valid when the following is called:
|
No. The only new (destination) object is valid after compact. The source location can be destroyed during compaction later on. Forwarding pointer (to new location) is stored in Compacted Mark Map and does not rely on heap content. |
Did you check that every single GC has discovered module object (even without updating of the pointer)? The theory was the object became stale (lost for one GC and re-discovered later) way earlier than pointer is damaged actually by compact. I guess one easy way to check it run test with |
Yes I verified that every GC after the module is created, it is scanned. |
The problem is: GC scans System and Application classloaders separately as a permanent roots. Scanning of modules for these classloaders has been missed. I will provide fix shortly. |
GC scans System and Application class loaders seperately as permanent roots. Scanning of the modules has been missed for this path. related eclipse-openj9#18805 Signed-off-by: Dmitri Pivkine <[email protected]>
GC scans System and Application class loaders seperately as permanent roots. Scanning of the modules has been missed for this path. related eclipse-openj9#18805 Signed-off-by: Dmitri Pivkine <[email protected]>
GC scans System and Application class loaders seperately as permanent roots. Scanning of the modules has been missed for this path. related eclipse-openj9#18805 Signed-off-by: Dmitri Pivkine <[email protected]>
We can unexclude the test and close this now that #19020 is merged? |
Fixed by eclipse-openj9/openj9#19020 Closes: eclipse-openj9/openj9#18804 Closes: eclipse-openj9/openj9#18805 Signed-off-by: Babneet Singh <[email protected]>
I had locally verified that #19020 fixes this issue. I have opened adoptium/aqa-tests#5116 to re-enable the test. |
Fixed by eclipse-openj9/openj9#19020 Closes: eclipse-openj9/openj9#18804 Closes: eclipse-openj9/openj9#18805 Signed-off-by: Babneet Singh <[email protected]>
https://openj9-jenkins.osuosl.org/job/Test_openjdk22_j9_sanity.openjdk_aarch64_linux_Personal_testList_1/1
jdk_lang
java/lang/invoke/MethodHandleProxies/WrapperHiddenClassTest.java
The text was updated successfully, but these errors were encountered: