-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Break the initialization cycle in NIO/cgroups code #7478
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
Conversation
First, we use a separate accessor for page-alignedness as it doesn't need the more sophisticated initialization of the directMemory field. Next, ensure PhysicalMemory initialization is serialized and when it is, set directMemory to a static value so that the container code can finish initialization without introducing a cyle. The final directMemory value based on the heap size is then published to JDK code by setting the VM init level to 1. Therefore, application code would use the non-static value as the upper bound. Closes: oracle#556
15fb3e9 to
1ae2dc0
Compare
|
This is an alternative fix to: |
|
@adinn @roberttoyonaga Could you perhaps review this, please? |
...tratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java
Outdated
Show resolved
Hide resolved
...tratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java
Outdated
Show resolved
Hide resolved
|
Thanks for the review, @roberttoyonaga! |
roberttoyonaga
left a comment
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!
christianhaeubl
left a comment
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 get the idea of the PR but I think it is too complex and at the same time not thread-safe enough. I implemented an alternative, see #7553. Please let me know if that simpler approach is good enough.
Thanks. I'll take a look later today and let you know. |
It's good enough to unbreak us. Thanks! Closing as the replacement PR is #7553. |
When building with an unpatched JDK 21, there is an initialization cycle in the native image code. The details are explained in this issue:
graalvm#556
This isn't easily reproducible with a LabsJDK 21 based build. However, I think it would be good to not be dependent on the patch in the base JDK. We have been using this patch in Mandrel as we are using unpatched OpenJDK 21 as a base.
Details of the patch:
First, we use a separate accessor for page-alignedness as it doesn't need the more sophisticated initialization of the directMemory field.
Next, ensure PhysicalMemory initialization is serialized and when it is, set directMemory to a static value so that the container code can finish initialization without introducing a cyle. The final directMemory value based on the heap size is then published to JDK code by setting the VM init level to 1. Therefore, application code would use the non-static value as the upper bound.
Closes: graalvm#556