Inject the class loader from the parent context into the child context

[tommyk-gears]

ConditionEvaluator extracts the class loader from the bean factory at instantiation, and then uses said classloader to load classes when evaluating conditionals. Hence, we need to inject the parent class loader into a bean factory that we inject into the child context. This fix is an extension of the fix made in spring-cloud/spring-cloud-netflix#3101.

Fixes spring-cloud/spring-cloud-openfeign#475

[ryanjbaxter]

I assume this bug is present in 2021.0.x? If so you probably want to make this PR against 3.1.x and we will merge it forward to main.

[tommyk-gears]

I assume this bug is present in 2021.0.x? If so you probably want to make this PR against 3.1.x and we will merge it forward to main.

Yes, you are right. I updated the PR to target 3.1.x.

[ryanjbaxter]

@OlgaMaciaszek do you want to take a look at this since its related to OpenFeign?

[OlgaMaciaszek]

Hello @tommyk-gears, thanks a lot for submitting this fix. Looks good, have just added comments about some cosmetic changes; Also, please add your full name and surname with @author tag to the javadocs of the classes that you've modified.

[tommyk-gears]

Hello @tommyk-gears, thanks a lot for submitting this fix. Looks good, have just added comments about some cosmetic changes; Also, please add your full name and surname with @author tag to the javadocs of the classes that you've modified.

Thanks! I pushed a commit addressing you comments. I also found an accidental usage of Java9 API (ClassLoader constructor with name parameter) that I removed.

[OlgaMaciaszek]

Thanks, @tommyk-gears - we should have caught it - it seems there's an issue with our CI builds for commons.

[OlgaMaciaszek]

Thanks, @tommyk-gears. LGTM.

[OlgaMaciaszek]

@tommyk-gears I have tried retesting the OF issue after your changes using this updated sample branch, but I still get the same issue, so it does not seem this fixes it.

[tommyk-gears]

I'm afraid you are right @OlgaMaciaszek - the fix is to reuse the class loader from the parent context in the child context, but if the parent context does not have a class loader explicitly set, then parent#getClassLoader() in practice just returns the TCCL of the current thread - which is the same class loader that we would use anyway.
My fix is based on the (mostly false) assumption that parent#getClassLoader() would return a "useful" class loader.

I successfully tried to explicitly set the class loader of the parent context in an ApplicationContextInitializer (needs to be registered in META-INF/spring.factories) like this;

class ClassLoaderApplicationContextInitializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {

    private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderApplicationContextInitializer.class);

    @Override
    public void initialize(ConfigurableApplicationContext applicationContext) {
        // Keep using the current class loader if one exists...
        ClassLoader classLoader = applicationContext.getClassLoader();
        if (classLoader == null) {
            // ... if not, try to get a default class loader
            classLoader = ClassUtils.getDefaultClassLoader();
        }
        if (classLoader != null) {
            LOG.debug("Using {} as application context classloader in {}", classLoader, applicationContext);
            applicationContext.setClassLoader(classLoader);
        }
    }
}

With the above "hack" + the fix in this issue, lazy init of feign clients works also on threads with a less useful TCCL. From my POV I could probably live with implementing my hack into our own services, but I understand that it is probably not going into the Spring framework. I honestly do not know how to move forward with this. The only source of a "good" class loader when creating the child context is the beanClassLoader of the beanFactory in the parent context - but it seems a bit odd to use the parent beanClassLoader and use it as both resource class loader and bean class loader in the child context.

Btw, regarding spring-cloud/spring-cloud-openfeign#475 it might be worth noting that the issue is reproducible both with docker image and with fat jar.

[HJK181]

@OlgaMaciaszek is there any way/chance that the issue really gets fixed? The solution of @tommyk-gears can't be the proposed one, right? The class loader problem is a blocker for us updating from Spring 2.3/Java11 to Spring 2.7/Java17.

[OlgaMaciaszek]

Yes @HJK181 , we will work on a fix for this. We're now fully on the upcoming 2022.x release, but once the priorities of that release are handled, this is going to be the first issue on my queue.

[HJK181]

@OlgaMaciaszek Shouldn't this also fix gh-475?

I'm still facing java.lang.ClassNotFoundException: org.springframework.boot.autoconfigure.condition.OnPropertyCondition with spring-cloud-commons:3.1.4 as described in gh-475.

[OlgaMaciaszek]

No, that's a different issue. Please, read the comment.