procyon icon indicating copy to clipboard operation
procyon copied to clipboard

Get ird of sun.misc.Unsafe.defineClass in favor of java.lang.invoke.MethodHandles.Lookup.defineClass

Open judovana opened this issue 5 years ago • 7 comments

https://bugs.openjdk.java.net/browse/JDK-8181442 As it is jdk9+ only, maybe to do so via reflection, or by some if-def class? Afaik it is the onyly way to move to jdk11. Would be a potty to lost procyon once jdk8 eols.

I tried to go with https://src.fedoraproject.org/rpms/procyon/blob/master/f/lookupPatch.patch

But tbh, I do not have much clues if I did it right. Thoughts?

judovana avatar May 04 '20 14:05 judovana

This is a rather complicated issue that I've only just started addressing. Problem is, the Lookup-based alternative is much more restrictive (by design) and will require some refactoring. I have a bunch of production code I can use to test out various approaches. Hopefully I'll have some time to try them out in the coming weeks.

mstrobel avatar Oct 12 '20 14:10 mstrobel

For now, I think making an anonymous class loader delegating to the thread context class loader to load this class might be good enough.

liach avatar Jul 05 '21 01:07 liach

ok. WIll do the PR then! ty!

judovana avatar Jul 07 '21 19:07 judovana

Waiy. it is closed. But #17 do not seem to address it. What have I missed?

judovana avatar Jul 07 '21 19:07 judovana

Fyi #17 mentions this because I hope #17 would support building procyon with like java 16, but it won't as this unsafe defineclass is removed before java 12.

liach avatar Jul 08 '21 01:07 liach

Is there an official solution for JDK11+, to be used in conjunction with expressions? The proposed patch does not work (gradle 7.3.3 and JDK11) because MethodHandles.privateLookupIn(Class.forName(fullName), lookup) fails (the class name does not exist yet), and a class in the same package as the class to be generated is needed to use privateLookupIn.

The only way I was able to make it work, is declare a static ByteArrayClassLoader in TypeBuilder

    private static ByteArrayClassLoader _internalClassLoader;

    private static class ByteArrayClassLoader extends ClassLoader {
        private final Map<String, byte[]> extraClassDefs;

        public ByteArrayClassLoader(final ClassLoader parent) {
            super(parent);
            this.extraClassDefs = new HashMap<>();
        }

        public void declareClass(final String name, final byte[] bytes)
        {
            extraClassDefs.put(name, bytes);
        }

        @Override
        protected Class<?> findClass(final String name) throws ClassNotFoundException {
            final byte[] classBytes = this.extraClassDefs.remove(name);
            if (classBytes != null) {
                return defineClass(name, classBytes, 0, classBytes.length);
            }
            return super.findClass(name);
        }
    }

and replace the Unsafe code removed in the patch with

            if(_internalClassLoader == null)
                _internalClassLoader = new ByteArrayClassLoader(getClass().getClassLoader());
            _internalClassLoader.declareClass(fullName, classBytes);
            _generatedClass = (Class<T>)_internalClassLoader.loadClass(fullName);

This way I was able to run unit tests with JDK11, but it feels so hackish...

BladeWise avatar Jan 20 '22 19:01 BladeWise

Actually that sounds like an good fix. I keep wondering why I had never hit that issue. Do you mind to change it to PR? I will then include it as in-package patch for Fedora RPMs.

judovana avatar Jan 22 '22 21:01 judovana