Skip to content
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

Injecting bean in configurable class using load-time weaving broken when referenced on scoped-proxy class [SPR-14892] #19458

Closed
spring-issuemaster opened this issue Nov 9, 2016 · 7 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Nov 9, 2016

Holger Stenzhorn opened SPR-14892 and commented

In version 4.3.3 the following code to inject a bean into a Hibernate entity using load time weaving worked flawlessly and someBean is correctly set but after upgrading to version 4.3.4 someBean is now null.

@Configurable(preConstruction = true)
@Entity
public class ExampleEntity {
    @Inject SomeBean someBean;
    ...
}

Affects: 4.3.4

Issue Links:

  • #19382 Allow type produced by ScopedProxyFactoryBean to be determined before singleton is created
  • #19498 NPE in LoadTimeWeavingConfiguration: loadTimeWeaver() called too early
  • #19608 Regression with poolTargetSource and scoped proxy
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 11, 2016

Juergen Hoeller commented

In 4.3.4, we only really added an ApplicationListenerDetector as a BeanPostProcessor a bit earlier. I'm not sure how that could break load-time weaving...

If you're seeing null there it simply didn't get injected at all, so probably the ExampleEntity class got loaded too early and the aspect isn't weaved in at all.

Any chance you could compare when your class gets loaded with 4.3.3 vs 4.3.4, and which stacktrace actually triggers that?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 12, 2016

Holger Stenzhorn commented

I must admit that I was a little too quick when submitting this bug report initially. So I performed some further testing/debugging and now found the exact constellation when the issue occurs for me and when not.

With the code below, when I now call the method test() from AnotherBean then null is printed on the console - and it makes no difference whether I add @Scope("session") to SomeBean not, i.e. I get null in both cases.

But when I perform on AnotherBean one of the two following changes - or both together - then (e.g.) org.example.SomeBean@2c70812d is printed on the console:

  • Remove the annotation @Scope("session")
  • Remove the method dummy()

(Note again that this behavior is specific to version 4.3.4 as I always get the correct behavior that the object is printed on the console with version 4.3.3.)

package org.example;

import org.springframework.beans.factory.annotation.Configurable;
import javax.inject.Inject;

@Configurable
public class SomeConfigurable {
  @Inject public SomeBean someBean;
}
package org.example;

//import org.springframework.context.annotation.Scope;
import javax.inject.Named;

@Named
//@Scope("session")
public class SomeBean {}
package org.example;

import org.springframework.context.annotation.Scope;
import javax.inject.Named;

@Named
@Scope("session")
public class AnotherBean {
  public void test() { System.out.println(new SomeConfigurable().someBean); }
  public SomeConfigurable dummy() { return null; }
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 20, 2016

Juergen Hoeller commented

I'm afraid it is generally very fragile to declare load-time-weaved types in method signatures of other classes. Any attempt to introspect those methods will trigger loading of those classes, and if this happens before the LoadTimeWeaver and its corresponding ClassFileTransformers are set up, the class gets loaded in its raw form, with later-coming transformers not applying anymore. The only safe way out is compile-time weaving.

I haven't found a specific change in 4.3.4 that radically impacted the arrangement here. Our general introspection of method signatures, e.g. finding @Bean methods and deriving further bean definitions from them as early as possible, usually triggers the loading of types in method signatures; this was also the case in my local tests. From that perspective, I wonder why this worked for you in 4.3.3 at all; might have been accidental.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 20, 2016

Holger Stenzhorn commented

Well, albeit this might indeed be brittle, the actual code - i.e. not the above toy test class - had always worked fine since I had introduced it quite some time ago... So I checked my code again and I (seemingly) found the "culprit": I employ Java-based configuration and in the configuration class I use @ComponentScan(basePackages = "org.example", scopedProxy = TARGET_CLASS) where the part scopedProxy = TARGET_CLASS is needed since there are some circular bean dependencies in our code... Now, if the scopedProxy part is removed then the test case works fine also with 4.3.4.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 21, 2016

Holger Stenzhorn commented

As a follow-up to my previous comment: I have dogged a bit in the code changes between 4.3.3 and 4.3.4 and the cause of my issue is rooted in the change performed for #19382 in the class DefaultListableBeanFactory.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 28, 2016

Juergen Hoeller commented

This turns out to be due to a misguided isSingleton check which triggers creation of the ScopedProxyFactoryBean just to find out about its singleton status (which is always true). I've refined that check to avoid such a check in case of a decorated bean definition, restoring initialization behavior as you've experienced it with 4.3.3.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 28, 2016

Holger Stenzhorn commented

Great! Thanks a lot for fixing this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.