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

Remove IntrinsicCandidate annotation on non-native delegate methods. #393

Conversation

pderop
Copy link
Contributor

@pderop pderop commented Jan 29, 2024

Since JDK16, a new @IntrinsicCandidate annotation has been introduced; It indicates that an annotated method may be (but is not guaranteed to be) intrinsified.

For example, the native Unsafe.park method is annotated with it:

 @IntrinsicCandidate
 public native void park(boolean isAbsolute, long time);

So, when BlockHound is instrumenting a native method, it first renames the native method using a prefix.
For example, Unsafe.park is renamed to Unsafe.$$BlockHound$$_park
And then the origin method (Unsafe.park) is rewritten in order to delegate the call to the renamed method, but the origin method is also rewritten to be non-native, so we end up with something like this after instrumentation:

class Unsafe {
 ...
 public native void  $$BlockHound$$_park(boolean isAbsolute, long time);
 
 @IntrinsicCandidate
 public void park(boolean isAbsolute, long time) {
    // delegates the call to  $$BlockHound$$_park
 }

now, in JDK16 and JDK17, when CheckIntrinsicsflag is enabled (true by default), the "native" keyword was optional, but in this JDK 18 change, now, a warning message is logged in case a method annotated with @IntrinsicCandidate is not native, while it's present in internal JDK list of the native methods that can be intrinsified.

The warning seems to be not harmful, but this PR just tries to avoid it by removing the @IntrinsicCandidate annotation from the proxy/delegate method (from public void park non-native method).

Fixes #392

@pderop pderop force-pushed the avoid-intrinsic-warning-when-redefining-native-method branch from 1635f2a to 690b5cb Compare January 29, 2024 21:52
Copy link
Contributor

@Badbond Badbond left a comment

Choose a reason for hiding this comment

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

With the instructions given here, I can verify that the warning is now gone. Thank you @pderop. Happy to retest if you come up with an alternative solution.

@pderop
Copy link
Contributor Author

pderop commented Jan 30, 2024

@Badbond , thanks having tested this PR !

let's merge it.

@pderop pderop merged commit 5581929 into reactor:master Jan 30, 2024
2 checks passed
@pderop pderop deleted the avoid-intrinsic-warning-when-redefining-native-method branch January 30, 2024 21:46
@pderop pderop added the type/enhancement A general enhancement label Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using JDK 18 or greater makes the JVM log an issue while loading Unsafe.park
2 participants