Skip to content
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

[23.1] Cgroups initialization cycle fix for unpatched JDK 21+35 #569

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

jerboaa
Copy link
Collaborator

@jerboaa jerboaa commented Sep 13, 2023

Break the initialization cycle in NIO/cgroups code

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: #556

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
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 13, 2023
@jerboaa jerboaa requested a review from zakkak September 13, 2023 10:10
Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the comments and jerboaa#3 for a suggestion on how to achieve the same without a synchronized block.

Don't use volatile in DirectMemoryAccessors.isInitialized and don't use
synchronization as the outcome of those races should amount to the same
values.
@jerboaa jerboaa force-pushed the cgroups_23.1_backport branch from d8c648e to f6a60b2 Compare September 13, 2023 16:06
@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 13, 2023

@zakkak Please take another look. Thanks!

Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 14, 2023

Thanks for the review! CI looks good, so I'll merge.

@jerboaa jerboaa merged commit 9d5b338 into graalvm:mandrel/23.1 Sep 14, 2023
@zakkak zakkak added the release/noteworthy-feature A PR that is worth highlighting in the Changelog label Sep 14, 2023
@zakkak zakkak added this to the 23.1.0.0-Final milestone Sep 14, 2023
@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 14, 2023

I'll also propose this patch upstream at some point (probably next week).

@zakkak
Copy link
Collaborator

zakkak commented Sep 14, 2023

I'll also propose this patch upstream at some point (probably next week).

Yes, we need it for 24.0 as well, so getting it upstream would be the best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement. release/noteworthy-feature A PR that is worth highlighting in the Changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants