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

Requirement for Java 11 compatibility #843

Closed
raphw opened this Issue May 9, 2018 · 14 comments

Comments

Projects
None yet
5 participants
@raphw
Contributor

raphw commented May 9, 2018

Java 11 is going to be the first LTS version of Java and it will remove support for sun.misc.Unsafe::defineClass which is currently used by both Byte Buddy and cglib.

As of Java 11, this loading approach does no longer work but since Byte Buddy 1.8 there is a new way of loading classes that unfortunately requires some code changes in Spock as method handle lookups are call site sensitive: https://mydailyjava.blogspot.no/2018/04/jdk-11-and-proxies-in-world-past.html

You need to invoke the load method in ProxyBasedMockFactory to use a class loading strategy that depends on the current JVM's capabilities. The code looks like the following:

ClassLoadingStrategy<ClassLoader> strategy;
if (ClassInjector.UsingLookup.isAvailable()) {
  Class<?> methodHandles = Class.forName("java.lang.invoke.MethodHandles");
  Object lookup = methodHandles.getMethod("lookup").invoke(null);
  Method privateLookupIn = methodHandles.getMethod("privateLookupIn", 
      Class.class, 
      Class.forName("java.lang.invoke.MethodHandles$Lookup"));
  Object privateLookup = privateLookupIn.invoke(null, targetClass, lookup);
  strategy = ClassLoadingStrategy.UsingLookup.of(privateLookup);
} else if (ClassInjector.UsingReflection.isAvailable()) {
  strategy = ClassLoadingStrateg.Default.INJECTION;
} else {
  throw new IllegalStateException(“No code loading strategy available”);
}

The Method and the base lookup instance should be reused to avoid an overhead if the lookup strategy is available.

Also, using this strategy, you probably need to change the naming strategy for java.* types as you cannot define classes in a random package. Instead, put those types into a dedicated package that contains another class that you own and resolve the private lookup against this class. The class can for example be org.spockframework.loading.Hook and then name classes that start with java.* by org.spockframework.loading.

I am currently on parental leave without too much time. I will send you a PR if I get a quite moment but its not a big change, so maybe you can find the time to do it yourself.

@tburny

This comment has been minimized.

tburny commented Jul 14, 2018

Actually this is blocked by #808 which we need to solve first.
I successfully added a --release parameter locally so I can move on with this topic. (Side note: You also have to set source/target compatibility in IntellIJ to 1.8 or less because of #69)

Next I added the above snippet in org.spockframework.mock.runtime.ProxyBasedMockFactory.ByteBuddyMockFactory#createMock:

 ClassLoadingStrategy strategy;
      if (ClassInjector.UsingLookup.isAvailable()) {
        try {
          Class<?> methodHandles = Class.forName("java.lang.invoke.MethodHandles");
          Object lookup = methodHandles.getMethod("lookup").invoke(null);
          Method privateLookupIn = methodHandles.getMethod("privateLookupIn",
            Class.class,
            Class.forName("java.lang.invoke.MethodHandles$Lookup"));
          Object privateLookup = privateLookupIn.invoke(null, type, lookup);
          strategy = ClassLoadingStrategy.UsingLookup.of(privateLookup);
        } catch (ReflectiveOperationException e) {
          throw new SpockException(e);
        }
      }
    // Commented force the "lookup" strategy
    /* else if (ClassInjector.UsingReflection.isAvailable()) {
        strategy = ClassLoadingStrategy.Default.INJECTION;
      }*/ else {
        throw new IllegalStateException("No code loading strategy available");
      }

and added the strategy to the load() method as follows:

              .make()
              .load(classLoader, strategy)
              .getLoaded();

All tests seem to pass so far, but Spring and Tapestry are broken:

Spring

Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'someExistingBean' defined in class path resource [org/spockframework/spring/docs/MockDocu-context.xml]: Initialization of bean failed; nested exception is java.lang.IllegalArgumentException: Could not create type
Caught exception while allowing TestExecutionListener [org.springframework.test.context.support.DependencyInjectionTestExecutionListener@1103d6b5] to prepare test instance [org.spockframework.spring.docs.DetachedXmlExample@62d47cd0]
java.lang.IllegalStateException: Failed to load ApplicationContext
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:124)
	at org.springframework.test.context.support.DefaultTestContext.getApplicationContext(DefaultTestContext.java:83)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.injectDependencies(DependencyInjectionTestExecutionListener.java:117)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.prepareTestInstance(DependencyInjectionTestExecutionListener.java:83)
	at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:230)
	at org.spockframework.spring.SpringTestContextManager.prepareTestInstance(SpringTestContextManager.java:56)
	at org.spockframework.spring.SpringInterceptor.interceptInitializerMethod(SpringInterceptor.java:43)
	at org.spockframework.runtime.extension.AbstractMethodInterceptor.intercept(AbstractMethodInterceptor.java:24)
	at org.spockframework.runtime.extension.MethodInvocation.proceed(MethodInvocation.java:97)
	at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:475)
	at org.spockframework.runtime.BaseSpecRunner.runInitializer(BaseSpecRunner.java:341)
	at org.spockframework.runtime.BaseSpecRunner.runInitializer(BaseSpecRunner.java:336)
	at org.spockframework.runtime.BaseSpecRunner.initializeAndRunIteration(BaseSpecRunner.java:274)
	at org.spockframework.runtime.BaseSpecRunner.runSimpleFeature(BaseSpecRunner.java:266)
	at org.spockframework.runtime.BaseSpecRunner.doRunFeature(BaseSpecRunner.java:260)
	at org.spockframework.runtime.BaseSpecRunner$5.invoke(BaseSpecRunner.java:243)
	at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
	at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
	at org.spockframework.runtime.BaseSpecRunner.runFeature(BaseSpecRunner.java:235)
	at org.spockframework.runtime.BaseSpecRunner.runFeatures(BaseSpecRunner.java:185)
	at org.spockframework.runtime.BaseSpecRunner.doRunSpec(BaseSpecRunner.java:95)
	at org.spockframework.runtime.BaseSpecRunner$1.invoke(BaseSpecRunner.java:81)
	at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
	at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
	at org.spockframework.runtime.BaseSpecRunner.runSpec(BaseSpecRunner.java:73)
	at org.spockframework.runtime.BaseSpecRunner.run(BaseSpecRunner.java:64)
	at org.spockframework.runtime.Sputnik.run(Sputnik.java:63)
[etc]

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'someExistingBean' defined in class path resource [org/spockframework/spring/docs/MockDocu-context.xml]: Initialization of bean failed; nested exception is java.lang.IllegalArgumentException: Could not create type
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:563)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:483)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:759)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:866)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:542)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:128)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:60)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.delegateLoading(AbstractDelegatingSmartContextLoader.java:108)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.loadContext(AbstractDelegatingSmartContextLoader.java:251)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:98)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:116)
	... 56 more
Caused by: java.lang.IllegalArgumentException: Could not create type
	at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:139)
	at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:345)
	at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:160)
	at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:354)
	at org.spockframework.mock.runtime.ProxyBasedMockFactory$ByteBuddyMockFactory.createMock(ProxyBasedMockFactory.java:120)
	at org.spockframework.mock.runtime.ProxyBasedMockFactory.create(ProxyBasedMockFactory.java:60)
	at org.spockframework.mock.runtime.JavaMockFactory.createInternal(JavaMockFactory.java:58)
	at org.spockframework.mock.runtime.JavaMockFactory.createDetached(JavaMockFactory.java:43)
	at org.spockframework.mock.runtime.CompositeMockFactory.createDetached(CompositeMockFactory.java:54)
	at org.spockframework.mock.MockUtil.createDetachedMock(MockUtil.java:112)
	at spock.mock.DetachedMockFactory.createMock(DetachedMockFactory.java:169)
	at org.spockframework.spring.xml.SpockSpyBeanPostProcessor.postProcessAfterInitialization(SpockSpyBeanPostProcessor.java:53)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:423)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1594)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:554)
	... 70 more
Caused by: java.lang.IllegalArgumentException: class net.bytebuddy.renamed.java.util.ArrayList$SpockMock$syhbwXXi must be defined in the same package as java.util.ArrayList/package
	at net.bytebuddy.dynamic.loading.ClassInjector$UsingLookup.inject(ClassInjector.java:931)
	at net.bytebuddy.dynamic.loading.ClassLoadingStrategy$UsingLookup.load(ClassLoadingStrategy.java:390)
	at net.bytebuddy.dynamic.TypeResolutionStrategy$Passive.initialize(TypeResolutionStrategy.java:79)
	at net.bytebuddy.dynamic.DynamicType$Default$Unloaded.load(DynamicType.java:4225)
	at org.spockframework.mock.runtime.ProxyBasedMockFactory$ByteBuddyMockFactory$1.call(ProxyBasedMockFactory.java:141)
	at org.spockframework.mock.runtime.ProxyBasedMockFactory$ByteBuddyMockFactory$1.call(ProxyBasedMockFactory.java:122)
	at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:137)

Tapestry

Seems to break because of https://issues.apache.org/jira/browse/TAP5-2588

javax/annotation/PostConstruct
java.lang.NoClassDefFoundError: javax/annotation/PostConstruct
	at org.apache.tapestry5.ioc.internal.util.InternalUtils.extendPlanForPostInjectionMethods(InternalUtils.java:1502)
	at org.apache.tapestry5.ioc.internal.util.InternalUtils.access$300(InternalUtils.java:50)
	at org.apache.tapestry5.ioc.internal.util.InternalUtils$20.invoke(InternalUtils.java:1382)
	at org.apache.tapestry5.ioc.internal.util.InternalUtils$20.invoke(InternalUtils.java:1366)
	at org.apache.tapestry5.ioc.internal.OperationTrackerImpl.invoke(OperationTrackerImpl.java:82)
	at org.apache.tapestry5.ioc.internal.PerThreadOperationTracker.invoke(PerThreadOperationTracker.java:72)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.invoke(RegistryImpl.java:1260)
	at org.apache.tapestry5.ioc.internal.util.InternalUtils.createConstructorConstructionPlan(InternalUtils.java:1363)
	at org.apache.tapestry5.ioc.internal.ConstructorServiceCreator.getPlan(ConstructorServiceCreator.java:52)
	at org.apache.tapestry5.ioc.internal.ConstructorServiceCreator.createObject(ConstructorServiceCreator.java:62)
	at org.apache.tapestry5.ioc.internal.ModuleImpl$4.invoke(ModuleImpl.java:295)
	at org.apache.tapestry5.ioc.internal.OperationTrackerImpl.invoke(OperationTrackerImpl.java:82)
	at org.apache.tapestry5.ioc.internal.PerThreadOperationTracker.invoke(PerThreadOperationTracker.java:72)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.invoke(RegistryImpl.java:1260)
	at org.apache.tapestry5.ioc.internal.ModuleImpl.create(ModuleImpl.java:344)
	at org.apache.tapestry5.ioc.internal.ModuleImpl.access$100(ModuleImpl.java:40)
	at org.apache.tapestry5.ioc.internal.ModuleImpl$1.invoke(ModuleImpl.java:198)
	at org.apache.tapestry5.ioc.internal.util.ConcurrentBarrier.withWrite(ConcurrentBarrier.java:139)
	at org.apache.tapestry5.ioc.internal.ModuleImpl$2.invoke(ModuleImpl.java:215)
	at org.apache.tapestry5.ioc.internal.util.ConcurrentBarrier.withRead(ConcurrentBarrier.java:83)
	at org.apache.tapestry5.ioc.internal.ModuleImpl.findOrCreate(ModuleImpl.java:221)
	at org.apache.tapestry5.ioc.internal.ModuleImpl.getService(ModuleImpl.java:112)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.getService(RegistryImpl.java:497)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.getServiceByTypeAlone(RegistryImpl.java:783)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.getServiceByTypeAndMarkers(RegistryImpl.java:797)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.getService(RegistryImpl.java:755)
	at org.apache.tapestry5.ioc.internal.RegistryImpl.<init>(RegistryImpl.java:212)
	at org.apache.tapestry5.ioc.RegistryBuilder.build(RegistryBuilder.java:181)
	at org.spockframework.tapestry.TapestryInterceptor.createAndStartupRegistry(TapestryInterceptor.java:91)
	at org.spockframework.tapestry.TapestryInterceptor.interceptSharedInitializerMethod(TapestryInterceptor.java:51)
	at org.spockframework.runtime.extension.AbstractMethodInterceptor.intercept(AbstractMethodInterceptor.java:27)
	at org.spockframework.runtime.extension.MethodInvocation.proceed(MethodInvocation.java:97)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:106)
[...more gradle internal stuff...]
@raphw

This comment has been minimized.

Contributor

raphw commented Jul 20, 2018

As for the Spring related test: You cannot normally define classes in JVM packages as a package needs to be opened to the Spock module (or the unnamed module) to allow definition within it. I would therefore recommend you to use ClassLoadingStrategy.Default.WRAPPER if the mock is in the java.* package. This looses you the ability to mock package-private types and to mock package-private methods of these classes but I assume that only few if any rely on this.

@MartyIX

This comment has been minimized.

Contributor

MartyIX commented Jul 27, 2018

@tburny I added PR #862. Maybe you have already done some work to improve it further or the other way around.

@tburny

This comment has been minimized.

tburny commented Aug 5, 2018

@MartyIX This is a great start!
I saw that you changed a lot of stuff along the way like updating versions of stuff - where some probably isn't necessary to archive Java 11 compatibility. Some of the stuff fixes really nasty problems, others look more unrelated to me and might accidentelly break stuff.
The path I chose was to change the absolute minimum of things, so the compromise would be to handle these cases in a seperate pull request 👍

I found out that it is easy to go the "lets refactor a bit here, change a bit there" path along the way, but it does make the changes (and the pull request) more complicated and confusing.
For instance I'd love to refactor the mocking proxy code a lot, as it is quite confusing. Still - I won't do that in this issue :)

@MartyIX

This comment has been minimized.

Contributor

MartyIX commented Aug 5, 2018

@tburny I fully agree. I'm still in the phase of "making it work".

@oehme

This comment has been minimized.

oehme commented Aug 6, 2018

Just adding my +1 here, since the Gradle team currently can't verify whether Gradle fully works on Java 11 because some of our tests use proxying and fail because of the missing Java 11 support.

@oehme oehme referenced this issue Aug 6, 2018

Closed

Java 11 support #5120

@MartyIX

This comment has been minimized.

Contributor

MartyIX commented Aug 24, 2018

@tburny I have cleaned up the commit a bit. :-)

@tburny

This comment has been minimized.

tburny commented Aug 24, 2018

@MartyIX great work! I had a chat with @leonard84 some time ago and as it seems Java 7 should still be supported as long as possible (Spock 2 will support Java 8 and I hope JUnit 5).
I added this in the comments, otherwise it looks amazing at first glance (although I'm not a contributor).

@MartyIX

This comment has been minimized.

Contributor

MartyIX commented Aug 24, 2018

@tburny Yes, that's my goal. It's not that easy. :-)

@leonard84

This comment has been minimized.

Member

leonard84 commented Aug 28, 2018

@raphw I've encountered some situations where it would be quite helpful to be able to mock package-private types/methods.
Is it feasible to have a Composite strategy, that tries the more powerful strategies first and then falls back to the less powerful strategies? Also is it possible to check if a strategy could be used to create a mock, or do we have to catch the exception and try the less powerful strategy?

@tburny yes Spock 1.x should stay Java 7 compatible. 2.x will be based on Java 8 and JUnit 5 Platform. However, 1.2 is the last planned release for 1.x, so development for 2.x will start soon™.

@MartyIX

This comment has been minimized.

Contributor

MartyIX commented Aug 29, 2018

(I've just added #885 PR to see how many tests fail with WRAPPER strategy)

@raphw

This comment has been minimized.

Contributor

raphw commented Aug 30, 2018

As described, if you resolve the method handle lookup against a class in this package, this is fully possible. It just requires that the class is opened to Spock. This is achieved via the privateLookupIn method.

By the way, I found a way to make the injection strategy work with Java 11. But this still relies on Unsafe, this can buy you some time if you want to delay the problem to Java 12. Doing so, all you need to do is to use the injection strategy explicitly and take the risk. Otherwise, the method handle lookup strategy does the trick. You can also take Mockito as a reference.

@leonard84

This comment has been minimized.

Member

leonard84 commented Aug 31, 2018

@raphw could we chat in gitter or the spock slack channel?

@raphw

This comment has been minimized.

Contributor

raphw commented Aug 31, 2018

I joined on Gitter.

marcphilipp added a commit to marcphilipp/spock that referenced this issue Sep 7, 2018

Use Lookup class loading strategy if available
In order to create mock objects on Java 11, `ProxyBasedMockFactory`
now uses ByteBuddy's `Lookup`-based `ClassLoadingStrategy`, if
available.

Resolves spockframework#843.

leonard84 added a commit that referenced this issue Sep 9, 2018

Use Lookup class loading strategy if available (#895)
In order to create mock objects on Java 11+, `ProxyBasedMockFactory`
now uses ByteBuddy's `Lookup`-based `ClassLoadingStrategy`, if
`Reflection` strategy is not available.

Resolves #843

MartyIX added a commit to MartyIX/spock that referenced this issue Oct 14, 2018

Use Lookup class loading strategy if available (spockframework#895)
In order to create mock objects on Java 11+, `ProxyBasedMockFactory`
now uses ByteBuddy's `Lookup`-based `ClassLoadingStrategy`, if
`Reflection` strategy is not available.

Resolves spockframework#843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment