Skip to content

Including spring-boot-devtools causes package-private getter on proxied class to return null #25367

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

Closed
jonatan-ivanov opened this issue Feb 19, 2021 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@jonatan-ivanov
Copy link
Member

This issue was originally created in Sleuth: spring-cloud/spring-cloud-sleuth#1630
I investigated it a little bit, you can see my findings here: spring-cloud/spring-cloud-sleuth#1630 (comment)

Description:
It seems if the system class loader is wrapped (e.g.: spring-boot-devtools restarts the app using a RestartClassLoader), cglib produces incorrect ClassLoaderData causing proxied package-private methods returning null (public/protected is fine); please see the original issue and investigation details.

Here's a sample project (uses 2.3.6.RELEASE) that reproduces the issue: https://github.com/jonatan-ivanov/sleuth-gh-1630-minimal Please check the readme, the issue is hiding.

@wilkinsona
Copy link
Member

wilkinsona commented Feb 19, 2021

When the target class is loaded by one class loader and the proxy class is loaded by another, the proxy can't access anything on the target that's package-private. This happens because package-private access requires code to be located in the same package and to have been loaded by the same class loader. To fix this automatically, I wonder if proxy creation needs to be a bit smarter about the ClassLoader that is uses when DevTools is in the picture.

It's AnnotationAwareAspectJAutoProxyCreator that's in the picture here and it uses the same ClassLoader for all the proxies that it creates. When you're using Devtools, this is the RestartClassLoader. The sample can be made to work by using a custom ProxyCreator that uses the target class's ClassLoader to create the proxy. Doing so requires some abuse of AnnotationAwareAspectJAutoProxyCreator's extensibility as it wasn't really designed for this sort of customization:

static class BeanClassClassLoaderAnnotationAwareAspectJAutoProxyCreator extends AnnotationAwareAspectJAutoProxyCreator {

    @Override
    protected Object createProxy(Class<?> beanClass, String beanName, Object[] specificInterceptors,
            TargetSource targetSource) {
        if (getBeanFactory() instanceof ConfigurableListableBeanFactory) {
            // Package-private so can't call this
            // AutoProxyUtils.exposeTargetClass((ConfigurableListableBeanFactory) getBeanFactory(), beanName, beanClass);
        }

        ProxyFactory proxyFactory = new ProxyFactory();
        proxyFactory.copyFrom(this);

        if (!proxyFactory.isProxyTargetClass()) {
            if (shouldProxyTargetClass(beanClass, beanName)) {
                proxyFactory.setProxyTargetClass(true);
            }
            else {
                evaluateProxyInterfaces(beanClass, proxyFactory);
            }
        }

        Advisor[] advisors = buildAdvisors(beanName, specificInterceptors);
        proxyFactory.addAdvisors(advisors);
        proxyFactory.setTargetSource(targetSource);
        customizeProxyFactory(proxyFactory);

        proxyFactory.setFrozen(isFrozen());
        if (advisorsPreFiltered()) {
            proxyFactory.setPreFiltered(true);
        }

        ClassLoader classLoader = beanClass.getClassLoader();
        System.out.println("Proxying " + beanClass + " using ClassLoader " + classLoader);
        return proxyFactory.getProxy(classLoader);
    }
    
};

It can then be hacked into place using a BFPP:

@Bean
static BeanFactoryPostProcessor hack() {
    return (beanFactory) -> {
        BeanDefinition definition = beanFactory.getBeanDefinition(AopConfigUtils.AUTO_PROXY_CREATOR_BEAN_NAME);
        if (AnnotationAwareAspectJAutoProxyCreator.class.getName().equals(definition.getBeanClassName())) {
            definition.setBeanClassName(BeanClassClassLoaderAnnotationAwareAspectJAutoProxyCreator.class.getName());
        }
    };
}

With these changes in place, the sample works with DemoService being proxied using the RestartClassLoader and the circuit break factory being proxied using the app class loader:


  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v2.3.6.RELEASE)

2021-02-19 10:45:22.976  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoApplication         : No active profile set, falling back to default profiles: default
2021-02-19 10:45:23.479  INFO [,,,] 62607 --- [  restartedMain] o.s.cloud.context.scope.GenericScope     : BeanFactory id=35769cb9-81fd-3224-81f3-bacd1a194697
Proxying class com.example.demo.DemoService using ClassLoader org.springframework.boot.devtools.restart.classloader.RestartClassLoader@6292fa3e
2021-02-19 10:45:24.068  INFO [,,,] 62607 --- [  restartedMain] o.s.b.d.a.OptionalLiveReloadServer       : LiveReload server is running on port 35729
Proxying class org.springframework.cloud.circuitbreaker.resilience4j.Resilience4JCircuitBreakerFactory using ClassLoader sun.misc.Launcher$AppClassLoader@2a139a55
2021-02-19 10:45:24.435  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoApplication         : Started DemoApplication in 1.997 seconds (JVM running for 2.426)
2021-02-19 10:45:24.438  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoAspect              : beforeNow
2021-02-19 10:45:24.445  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoApplication         : now: 2021-02-19T10:45:24.445Z
2021-02-19 10:45:24.448  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoAspect              : beforeStartedAt
2021-02-19 10:45:24.448  INFO [,,,] 62607 --- [  restartedMain] com.example.demo.DemoApplication         : startedAt: 2021-02-19T10:45:23.731Z

@wilkinsona
Copy link
Member

wilkinsona commented Feb 19, 2021

The problem can also be avoided by pulling spring-cloud-circuitbreaker-resilience4j up into the restart class loader using a META-INF/spring-devtools.properties file:

restart.include.cbr4j=.*/spring-cloud-circuitbreaker-resilience4j-[\\w\\d-\\.]+\\.jar

Ideally, users shouldn't have to know about this and it's only necessary when Sleuth is in the picture as it triggers the proxying of Resilience4JCircuitBreakerFactory. If a general purpose solution in Boot isn't possible, Sleuth could, in theory, package a META-INF/spring-devtools.properties file with the necessary restart.include configuration. That would allow the method's package-private scope to be restored.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Feb 23, 2021
@philwebb philwebb added status: blocked An issue that's blocked on an external project change type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Feb 24, 2021
@philwebb philwebb added this to the 2.4.x milestone Feb 24, 2021
@wilkinsona wilkinsona removed the status: blocked An issue that's blocked on an external project change label Feb 26, 2021
@wilkinsona wilkinsona self-assigned this Mar 10, 2021
@wilkinsona wilkinsona changed the title Including spring-boot-devtools causes package-private getter to return null Including spring-boot-devtools causes package-private getter on proxied class to return null Mar 10, 2021
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.4.4 Mar 10, 2021
wilkinsona added a commit that referenced this issue Mar 10, 2021
Closes gh-25584
Fixes gh-25367 in 2.5.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants