Skip to content

Commit

Permalink
OWB-1392 Use a different service to avoid breaking users by adding a …
Browse files Browse the repository at this point in the history
…new method.

It also allows a clear separation of concerns
  • Loading branch information
jeanouii committed Oct 6, 2021
1 parent 2af6184 commit 2428122
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.webbeans.exception.WebBeansException;
import org.apache.webbeans.hash.XxHash64;
import org.apache.webbeans.spi.DefiningClassService;
import org.apache.webbeans.spi.InstantiatingClassService;
import org.apache.xbean.asm9.ClassReader;
import org.apache.xbean.asm9.ClassWriter;
import org.apache.xbean.asm9.MethodVisitor;
Expand All @@ -62,6 +63,7 @@ public abstract class AbstractProxyFactory
protected final Unsafe unsafe;

private final DefiningClassService definingService;
private final InstantiatingClassService instantiatingService;

private final boolean useStaticNames;
private final boolean useXXhash64;
Expand All @@ -84,11 +86,19 @@ protected AbstractProxyFactory(WebBeansContext webBeansContext)
this.webBeansContext = webBeansContext;
javaVersion = determineDefaultJavaVersion();
definingService = webBeansContext.getService(DefiningClassService.class);

// if defining service implements both, we don't need to lookup the second service
instantiatingService = (definingService instanceof InstantiatingClassService)
? (InstantiatingClassService) definingService
: webBeansContext.getService(InstantiatingClassService.class);

useStaticNames = Boolean.parseBoolean(webBeansContext.getOpenWebBeansConfiguration()
.getProperty("org.apache.webbeans.proxy.useStaticNames"));
useXXhash64 = Boolean.parseBoolean(webBeansContext.getOpenWebBeansConfiguration()
.getProperty("org.apache.webbeans.proxy.staticNames.useXxHash64"));
unsafe = definingService == null ? new Unsafe() : null;

// we have fallbacks bellow to try Unsafe anyways if we can't do otherwise
unsafe = definingService == null || instantiatingService == null ? new Unsafe() : null;
}

private int determineDefaultJavaVersion()
Expand Down Expand Up @@ -338,9 +348,9 @@ protected <T> Class<T> createProxyClass(ClassLoader classLoader, String proxyCla

protected <T> T newInstance(final Class<? extends T> proxyClass)
{
if (definingService != null)
if (instantiatingService != null)
{
return definingService.newInstance(proxyClass);
return instantiatingService.newInstance(proxyClass);
}
return unsafe.unsafeNewInstance(proxyClass);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@
import org.apache.webbeans.exception.WebBeansException;
import org.apache.webbeans.logger.WebBeansLoggerFacade;
import org.apache.webbeans.spi.DefiningClassService;
import org.apache.webbeans.spi.InstantiatingClassService;

public class ClassLoaderProxyService implements DefiningClassService
public class ClassLoaderProxyService implements DefiningClassService, InstantiatingClassService
{
private final ProxiesClassLoader loader;

Expand Down Expand Up @@ -124,7 +125,7 @@ public <T> Class<T> defineAndLoad(final String name, final byte[] bytecode, fina
}

// strict load only impl, it changes LoadFirst by not creating a classloader at all (nice in graalvm) -@Experimental
public static class LoadOnly implements DefiningClassService
public static class LoadOnly implements DefiningClassService, InstantiatingClassService
{
@Override
public ClassLoader getProxyClassLoader(final Class<?> forClass)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,13 @@ public void defineInProxy() throws NoSuchMethodException
final ClassLoader proxyLoader = context.getService(DefiningClassService.class).getProxyClassLoader(proxyClass);
assertEquals(proxyLoader, proxyClass.getClassLoader());
proxyClass.getMethod("ok", String.class); // this line would fail if not here, no assert needed

// when using ClassLoaderProxyService, we don't use Unsafe to allocate the instance
// the regular reflection method newInstance is called and therefore the constructor gets called
// final Bean<MyBean> bean =
// final MyBean beanInstance = factory.createProxyInstance(proxyClass, factory.getInstanceProvider(proxyLoader, bean));
// assertTrue(beanInstance.isConstructorInvoked);
}

public static class MyBean
{
private final boolean constructorInvoked;

public MyBean() {
this.constructorInvoked = true;
}

public String ok(final String value)
{
return ">" + value + "<";
}

public boolean isConstructorInvoked() {
return constructorInvoked;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,4 @@ public interface DefiningClassService
* @return the proxy class
*/
<T> Class<T> defineAndLoad(String name, byte[] bytecode, Class<T> proxiedClass);

/**
* Create a new instance for a given proxy class.
* @param proxyClass the proxy class
* @param <T> type of the class to proxy
* @return the proxy instance
*/
<T> T newInstance(final Class<? extends T> proxyClass); // maybe a default method would make sense here
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.webbeans.spi;

/**
* SPI interface to define the logic to allocate/instantiate proxy instances.
* We used to do that using Unsafe.allocateInstance, but better to give the choice
*/
public interface InstantiatingClassService
{
/**
* Create a new instance for a given proxy class.
* @param proxyClass the proxy class
* @param <T> type of the class to proxy
* @return the proxy instance
*/
<T> T newInstance(final Class<? extends T> proxyClass);
}

0 comments on commit 2428122

Please sign in to comment.