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

Mark resolution of jdk.internal.misc as optional (again) #597

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

tobias--
Copy link

jdk.internal.misc was marked optional in #408. I think this resolution attribute was unintentionally removed, when jvm.driver was added with this commit: 4c30481

@lukehutch lukehutch merged commit 68ed4a3 into classgraph:latest Nov 12, 2021
@lukehutch
Copy link
Member

Thanks, sorry about that...

Out of curiosity, what broke due to this change?

@lukehutch
Copy link
Member

Fixed and released in 4.8.132

@tobias--
Copy link
Author

I tried using this bundle in an osgi environment for the first time. Resolving using bndtools failed, since bnd did not find any bundle that exports jdk.internal.misc. So I checked whether the package jdk.internal.misc has always been required.

Is this package actually required by classgraph? I did not find any usage of this package. The "internal" also suggests that it is not supposed to be used by third parties.

@lukehutch
Copy link
Member

The sun.misc.Unsafe class is used by ClassGraph. Unsafe is used to unmap mapped ByteBuffers as soon as they are no longer needed, so that file handles are closed as early as possible on Windows (otherwise temporary files can't be deleted). There is a class jdk.internal.misc.Unsafe that just proxies calls to the Sun class. ClassGraph used to try to use the jdk version of the class if it was available, but if I remember right, the jdk version was missing some critical functionality that was in the sun version, so I dropped support for the jdk version, and just use the sun version. I must have forgotten to remove the jdk package from the Manifest.

https://github.com/classgraph/classgraph/blob/latest/src/main/java/nonapi/io/github/classgraph/utils/FileUtils.java#L499

The sun version of Unsafe looks like it will stay in place until both versions of this class are completely removed from the JDK, probably around JDK 21 or so would be my guess -- so using the sun version should be sufficient.

I guess I should remove this entry from the manifest entirely!

@lukehutch
Copy link
Member

Sorry, there was a regression in 4.8.132, please try 4.8.133 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants