javaee / hk2

A light-weight and dynamic dependency injection framework
https://javaee.github.io/hk2
Other
112 stars 83 forks source link

HK2 attempts to inject generated lambda method, thinking it's an initializer #342

Closed glassfishrobot closed 8 years ago

glassfishrobot commented 8 years ago

My class uses a custom injection annotation to inject an argument into it's constructor. That argument is then referenced within a lambda expression, along with some other variables. The lambda expression causes a "lambda$new$1" method to be generated on my class, which has my injected argument as one of the parameters, including it's custom injection annotation.

When the class is being instantiated, org.jvnet.hk2.internal.Utilities.findInitializerMethods() reflection sees this lambda method, tests it first for hasInjectAnnotation() and finds my custom annotation, then tests it with Utilities.isProperMethod(), which also returns true, since it's not static or abstract, and thus the generated lambda method is considered an initializer method.

HK2 then thinks it must also inject arguments to the lambda method in order to initialize my class, and that fails due to all the arguments not being resolvable.

Environment

openjdk version "1.8.0_72" OpenJDK Runtime Environment (build 1.8.0_72-b16) OpenJDK 64-Bit Server VM (build 25.72-b16, mixed mode)

Affected Versions

[2.4.0]

glassfishrobot commented 8 years ago

Reported by hubick

glassfishrobot commented 8 years ago

hubick said: Sorry, version is 2.4.0-b34 (provided by Jersey 2.22.2).

glassfishrobot commented 8 years ago

@jwells131313 said: I have a very simple test that uses an incoming parameter in a constructor in a lambda expression and I do not see this error.

Can you please provide a simple code example of what you are trying to do here that is not working?

This is my silly little class which seems to work just fine:

public class LambdaInConstructorService { private final int sum; private final int diff;

@Inject public LambdaInConstructorService(AAndB aAndB)

{ sum = doOperation((int a, int b) -> a + b, aAndB); diff = doOperation((int a, int b) -> a - b, aAndB); }

private int doOperation(Operation a, AAndB aAndB)

{ return a.doWork(aAndB.getA(), aAndB.getB()); }

public int getSum()

{ return sum; }

public int getDiff()

{ return diff; }

}

glassfishrobot commented 8 years ago

hubick said: I believe when you write a lambda expression, it generates a method containing the code for that expression as part of the bytecode for your class, and that generated method has, as it's arguments, all the variables referenced from within that lambda expression.

Your constructor accepts an 'AAndB aAndB' argument, but does not appear to actually reference that argument from within either lambda expression (you simply pass it directly as the second argument to doOperation), so 'AAndB aAndB' will not be added as one of the arguments to the methods generated for housing the code for those expressions.

Also, the injection annotation needs to be applied directly to the argument itself, not the entire constructor. That way, when the generated method reuses the type as declared in the argument to the constructor, it brings that annotation with it...

public class LambdaInConstructorService { private final int value;

public LambdaInConstructorService(@Context final Supplier randomSupplier)

{ final List intList = Arrays.asList(1, 2, 3); value = doReduce(intList, (Integer a, Integer b) -> randomSupplier.get() + intList.get(0) + a + b); }

private int doReduce(final List integers, final BinaryOperator workFunc)

{ return integers.stream().reduce(0, workFunc); }

public int getValue()

{ return value; }

}

I haven't tested this, but here, I reference the injected 'randomSupplier' from within my lambda expression (which implements BinaryOperator), so randomSupplier should become one of the arguments to the generated function housing the code for that lambda, and should also reuse randomSupplier's datatype, including it's @Context annotation, which causes the generated function to be interpreted as an initializer.

Note that I have also purposefully added an extra reference to 'intList' from within my lambda expression to hopefully help guarantee that the generated method is receiving some argument HK2 will actually fail to inject, since if the randomSupplier injection to the constructor succeeded, having that being the only argument injected into the generated "initializer" might just happen to work (though 'a' and 'b' should also fail).

Does that help?

glassfishrobot commented 8 years ago

hubick said: I will mention that the compiler version may also matter here. I wasn't seeing this problem running the same code built on my old Fedora 20 box, but building it on my new Fedora 23 box with a newer OpenJDK release does exhibit the behaviour. I'm wondering if javac never used to carry over annotations into the generated method arguments, and was later changed to start doing that, or something?

glassfishrobot commented 8 years ago

@jwells131313 said: I've added a use case that works fine just like your code above. See:

https://github.com/hk2-project/hk2/blob/master/hk2-locator/src/test/java/org/glassfish/hk2/tests/locator/lambda/LambdaInConstructorService2.java

That being said, there was something I noticed that was "funny" in that you can't add an InjectionResolver and something that needs to be resolved in that same commit if hk2 is doing automatic class analysis. I wonder if you are running into this in your use case. This problem may be something I can fix, though I'd fix it under a different bug. To get around this, I made sure my resolver and the classes being automatically analyzed by hk2 were in separate commits. You can see that in the test case itself:

https://github.com/hk2-project/hk2/blob/master/hk2-locator/src/test/java/org/glassfish/hk2/tests/locator/lambda/LambdaTest.java

Here are some other notes on lambda's that seem to be incorrect in your writing above: 1. lambda's are just like anonymous inner classes, in that a class is generated with an implementation for an interface (one that has exactly one abstract method) 2. Because of the above, lambda's are kind of nothing more than "syntactic sugar" and really should have little to do with annotations and the like 3. There definitely could be errors in JDK implementations that might cause problems

Given the above facts about lambda's I think hk2 should (generally) be ok with the use of lambda's, especially how you are showing them in your example. If you have a test use case that fails, I'd be happy to take a look at it.

glassfishrobot commented 8 years ago

hubick said: I'm sorry I don't have a simpler use case for this. Do you have Git and Maven?

[hubick@CHWorkstation ~]$ mkdir tmp [hubick@CHWorkstation ~]$ cd tmp [hubick@CHWorkstation tmp]$ git clone https://github.com/www-eee/www-eee-util-misc.git [hubick@CHWorkstation tmp]$ cd www-eee-util-misc/ [hubick@CHWorkstation www-eee-util-misc]$ mvn install [hubick@CHWorkstation www-eee-util-misc]$ cd .. [hubick@CHWorkstation tmp]$ git clone https://github.com/www-eee/www-eee-portal.git [hubick@CHWorkstation tmp]$ cd www-eee-portal/main/

You can see that I've included a workaround for this bug to the code by adding a 'configPropsUnannotated' variable in the constructor...

[hubick@CHWorkstation main]$ cat main/src/main/java/net/www_eee/portal/WWWEEEPortal.java | grep Unannotated final RSProperties configPropsUnannotated = configProps; //TODO https://java.net/jira/browse/HK2-298 RSProperties.updateHierarchy(properties, propMap.values(), configPropsUnannotated);

Modify the file to remove the single use of that variable from within the lambda expression...

[hubick@CHWorkstation main]$ sed -i s/configPropsUnannotated)/configProps)/ main/src/main/java/net/www_eee/portal/WWWEEEPortal.java

Verify the reference has been removed...

[hubick@CHWorkstation main]$ cat main/src/main/java/net/www_eee/portal/WWWEEEPortal.java | grep Unannotated final RSProperties configPropsUnannotated = configProps; //TODO https://java.net/jira/browse/HK2-298

[hubick@CHWorkstation main]$ mvn install [hubick@CHWorkstation main]$ ls test-context/target/*.war test-context/target/www-eee-portal-main-test-context-0.5.0-SNAPSHOT.war

If I deploy that war into Tomcat and load http://localhost:8080/www-eee-portal-main-test/content/ I get...

javax.servlet.ServletException: A MultiException has 3 exceptions. They are: 1. org.glassfish.hk2.api.UnsatisfiedDependencyException: There was no object available for injection at SystemInjecteeImpl(requiredType=PropertyChangeEvent,parent=WWWEEEPortal,qualifiers={},position=2,optional=false,self=false,unqualified=null,1122832909) 2. java.lang.IllegalArgumentException: While attempting to resolve the dependencies of net.www_eee.portal.WWWEEEPortal errors were found 3. java.lang.IllegalStateException: Unable to perform operation: resolve on net.www_eee.portal.WWWEEEPortal org.glassfish.jersey.servlet.WebComponent.serviceImpl(WebComponent.java:489) org.glassfish.jersey.servlet.WebComponent.service(WebComponent.java:427) org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:388) org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:341) org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:228) org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52) net.www_eee.util.misc.servlet.role.KnownUnknownUserRoleFilter.doFilter(KnownUnknownUserRoleFilter.java:71) net.www_eee.util.misc.servlet.role.RequestHeaderUserRoleFilter.doFilter(RequestHeaderUserRoleFilter.java:102) net.www_eee.util.misc.servlet.role.LocaleUserRoleFilter.doFilter(LocaleUserRoleFilter.java:58)

If I leave the workaround, it's fine.

And, again, I'm using the latest openjdk, version "1.8.0_72", on Fedora 23.

glassfishrobot commented 8 years ago

@jwells131313 said: I was running the hk2 build for a completely unrelated issue on an older linux box and ran into this same problem!

It has been fixed by ignoring any synthetic construct or bridge method. Once I fixed that the test case worked fine

glassfishrobot commented 8 years ago

hubick said: Excellent! Much Thanks!

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA HK2-298

glassfishrobot commented 8 years ago

Marked as fixed on Monday, April 18th 2016, 6:39:58 am