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

3 EJB30 test failures in Platform TCK 8.0.2 when run in JDK11+ #1194

Closed
alwin-joseph opened this issue Oct 13, 2023 · 10 comments
Closed

3 EJB30 test failures in Platform TCK 8.0.2 when run in JDK11+ #1194

alwin-joseph opened this issue Oct 13, 2023 · 10 comments

Comments

@alwin-joseph
Copy link
Contributor

Describe the bug

Below 3 tests in JakartaEE8 Platform TCK are failing on JDK11+
com/sun/ts/tests/ejb30/lite/view/equals/Client.java#singletonEquals_from_ejbembed
com/sun/ts/tests/ejb30/lite/view/equals/Client.java#statefulEquals_from_ejbembed
com/sun/ts/tests/ejb30/lite/view/equals/Client.java#statelessEquals_from_ejbembed

TCK Bundle : https://www.eclipse.org/downloads/download.php?file=/jakartaee/platform/8/jakarta-jakartaeetck-8.0.2.zip
JDK versions: JDK 11, JDK 17
Implementation being tested : Weblogic Server 14.1.2
Weld Jar/Version used: 3.1.9.Final (https://mvnrepository.com/artifact/org.jboss.weld/weld-core-impl/3.1.9.Final)

Test Log:

 <Jul 14, 2023, 6:11:15,682 PM Greenwich Mean Time> <Error> <Deployer> <BEA-149265> <Failure occurred in the execution of deployment request with ID "10951260780304" for task "0" on [partition-name: DOMAIN]. Error is: "weblogic.management.DeploymentException: CDI deployment failure:WELD-001524: Unable to load proxy class for bean Session bean [class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean with qualifiers [@Any @Default]; local interfaces are [StatelessEqualsBean, BusinessLocalIF1, AnnotatedLocalBusinessInterface1, BusinessLocalIF2] with class class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean"
[exec] [javatest.batch] weblogic.management.DeploymentException: CDI deployment failure:WELD-001524: Unable to load proxy class for bean Session bean [class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean with qualifiers [@Any @Default]; local interfaces are [StatelessEqualsBean, BusinessLocalIF1, AnnotatedLocalBusinessInterface1, BusinessLocalIF2] with class class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean
[exec] [javatest.batch]
at com.oracle.injection.integration.CDIAppDeploymentExtension.initCdi(CDIAppDeploymentExtension.java:103)
[exec] [javatest.batch]
at com.oracle.injection.integration.CDIAppDeploymentExtension.activate(CDIAppDeploymentExtension.java:50)
[exec] [javatest.batch]
at weblogic.application.internal.flow.AppDeploymentExtensionFlow.activate(AppDeploymentExtensionFlow.java:39)
[exec] [javatest.batch]
at weblogic.application.internal.BaseDeployment$2.next(BaseDeployment.java:751)
[exec] [javatest.batch]
at weblogic.application.utils.StateMachineDriver.nextState(StateMachineDriver.java:45)
[exec] [javatest.batch]
Truncated.
[exec] [javatest.batch] Caused By: java.lang.IllegalArgumentException: com.sun.ts.tests.ejb30.common.busiface.StatelessEqualsBean$AnnotatedLocalBusinessInterface1$BusinessLocalIF1$BusinessLocalIF2$2037039882$Proxy$_$$_Weld$EnterpriseProxy$ not in same package as lookup class
[exec] [javatest.batch]
at java.base/java.lang.invoke.MethodHandleStatics.newIllegalArgumentException(MethodHandleStatics.java:167)
[exec] [javatest.batch]
at java.base/java.lang.invoke.MethodHandles$Lookup$ClassFile.newInstance(MethodHandles.java:2283)
[exec] [javatest.batch]
at java.base/java.lang.invoke.MethodHandles$Lookup.makeClassDefiner(MethodHandles.java:2318)
[exec] [javatest.batch]
at java.base/java.lang.invoke.MethodHandles$Lookup.defineClass(MethodHandles.java:1843)
[exec] [javatest.batch]
at org.jboss.weld.bean.proxy.util.WeldDefaultProxyServices.defineWithMethodLookup(WeldDefaultProxyServices.java:163)
[exec] [javatest.batch]
Truncated.

Additional context

  1. Moving 3 interfaces com.sun.ts.tests.ejb30.common.busiface.AnnotatedLocalBusinessInterface1, BusinessLocalIF1 and BusinessLocalIF2 to com.sun.ts.tests.ejb30.lite.view.equals package in the TCK will let the tests pass.

  2. From printing the logs in weld jar it seems like weld gets the proxyPackage of the Session bean [class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean incorrectly as com.sun.ts.tests.ejb30.common.busiface (the package of the interface). A hardcoded fix to correct the package for this Session bean by using proxyPackage = bean.getBeanClass().getPackage().getName() (similar to https://github.com/WELD-2758 Proxies for non-public class-based beans should reside in bean's package if possible weld/core#2886) let the tests pass. But not sure which should be the right set of conditions where the proxyPackage needs to be modified.

cc @manovotn Would you be able to comment please.
I should be able to add and print logs in the weld jars if that is helpful.

Embeddable EJB Application ejbembed_vehicle_ejb.jar(bundled with platform TCK 8.0.2) is being used for this test.

unzip ejbembed_vehicle_ejb.jar

Archive: ejbembed_vehicle_ejb.jar
creating: META-INF/
inflating: META-INF/MANIFEST.MF
creating: com/
creating: com/sun/
creating: com/sun/ts/
creating: com/sun/ts/lib/
creating: com/sun/ts/lib/harness/
inflating: com/sun/ts/lib/harness/EETest$Fault.class
inflating: com/sun/ts/lib/harness/EETest$SetupException.class
inflating: com/sun/ts/lib/harness/EETest.class
inflating: com/sun/ts/lib/harness/ServiceEETest.class
creating: com/sun/ts/tests/
creating: com/sun/ts/tests/common/
creating: com/sun/ts/tests/common/vehicle/
inflating: com/sun/ts/tests/common/vehicle/EmptyVehicleRunner.class
inflating: com/sun/ts/tests/common/vehicle/VehicleClient.class
inflating: com/sun/ts/tests/common/vehicle/VehicleRunnable.class
inflating: com/sun/ts/tests/common/vehicle/VehicleRunnerFactory.class
creating: com/sun/ts/tests/common/vehicle/ejbembed/
inflating: com/sun/ts/tests/common/vehicle/ejbembed/EJBEmbedRunner.class
inflating: com/sun/ts/tests/common/vehicle/ejbembed/EmbeddableEJBProcess.class
inflating: com/sun/ts/tests/common/vehicle/ejbembed/InjectionResolver.class
creating: com/sun/ts/tests/common/vehicle/ejbliteshare/
inflating: com/sun/ts/tests/common/vehicle/ejbliteshare/EJBLiteClientIF.class
inflating: com/sun/ts/tests/common/vehicle/ejbliteshare/EJBLiteSecuredWebVehicleRunner.class
inflating: com/sun/ts/tests/common/vehicle/ejbliteshare/EJBLiteWebVehicleRunner.class
inflating: com/sun/ts/tests/common/vehicle/ejbliteshare/ReasonableStatus.class
creating: com/sun/ts/tests/ejb30/
creating: com/sun/ts/tests/ejb30/common/
creating: com/sun/ts/tests/ejb30/common/busiface/
inflating: com/sun/ts/tests/ejb30/common/busiface/AnnotatedLocalBusinessInterface1.class
inflating: com/sun/ts/tests/ejb30/common/busiface/BusinessLocalIF1.class
inflating: com/sun/ts/tests/ejb30/common/busiface/BusinessLocalIF2.class
creating: com/sun/ts/tests/ejb30/common/helper/
inflating: com/sun/ts/tests/ejb30/common/helper/Helper.class
inflating: com/sun/ts/tests/ejb30/common/helper/ServiceLocator.class
creating: com/sun/ts/tests/ejb30/common/lite/
inflating: com/sun/ts/tests/ejb30/common/lite/EJBLiteClientBase.class
inflating: com/sun/ts/tests/ejb30/common/lite/NumberEnum.class
inflating: com/sun/ts/tests/ejb30/common/lite/NumberIF.class
creating: com/sun/ts/tests/ejb30/lite/
creating: com/sun/ts/tests/ejb30/lite/view/
creating: com/sun/ts/tests/ejb30/lite/view/equals/
inflating: com/sun/ts/tests/ejb30/lite/view/equals/Client.class
inflating: com/sun/ts/tests/ejb30/lite/view/equals/HttpServletDelegate.class
inflating: com/sun/ts/tests/ejb30/lite/view/equals/SingletonEqualsBean.class
inflating: com/sun/ts/tests/ejb30/lite/view/equals/StatefulEqualsBean.class
inflating: com/sun/ts/tests/ejb30/lite/view/equals/StatelessEqualsBean.class

@manovotn
Copy link
Contributor

I am unable to reproduce this on WFLY with similar test so take the information below with a pinch of salt as I am unable to debug it.

@alwin-joseph what's your implementation of ProxyServices? It seems WFLY passes this because it has its own implementation of ProxyServices and from your exception I guess that you are using the default one (org.jboss.weld.bean.proxy.util.WeldDefaultProxyServices)?
The default one is there mainly for Weld SE although I have no idea (apart from WFLY) how other EE servers approach class defining for CDI. I would expect that servers use their own impls because they are likely to need to perform some CL handling anyway and in that case delegating to ClassLoader#defineClass is the most straightforward way to solve these issues.

Apart from this, the scenario occurs only because the EJB bean uses @Local to declare an interface it doesn't directly implement. This triggers(here) the createCompoundProxyName method in Weld that you noticed. Ultimately, Weld will attempt to use MethodHandles.Lookup#defineClass to define the class and fail because the lookup will be based on StatelessEqualsBean which will have different package then the bean being defined.

@alwin-joseph
Copy link
Contributor Author

I am unable to reproduce this on WFLY with similar test so take the information below with a pinch of salt as I am unable to debug it.

WFLY was running in JDK11+? JDK11/17 is where the issue is reproduced for WLS.

@alwin-joseph what's your implementation of ProxyServices? It seems WFLY passes this because it has its own implementation of ProxyServices and from your exception I guess that you are using the default one (org.jboss.weld.bean.proxy.util.WeldDefaultProxyServices)?

Yes org.jboss.weld.bean.proxy.util.WeldDefaultProxyServices is being used here. Will you be able to point me to the implementation of `ProxyServices in WFLY (https://github.com/wildfly/wildfly?) .

@manovotn
Copy link
Contributor

WFLY was running in JDK11+? JDK11/17 is where the issue is reproduced for WLS.

Yes, that version of Weld still had to use MR JAR release so you need JDK 11+ to have access to JPMS and modular approach to class defining via MethodHandles.Lookup.
Therefore, there are two versions of the default impl:

The main difference between classical ClassLoader and MethodHandles.Lookup is that ClassLoader#defineClass can define classes in (almost) arbitrary package whereas MethodHandles.Lookup can do so only in the package in which you did the lookup - that is the source of the problem you are seeing. Try debugging WeldDefaultProxyServices to see that.

Yes org.jboss.weld.bean.proxy.util.WeldDefaultProxyServices is being used here. Will you be able to point me to the implementation of `ProxyServices in WFLY (https://github.com/wildfly/wildfly?) .

Here is a link to the current version which is for Weld 5, but it's very similar.

@alwin-joseph
Copy link
Contributor Author

createCompoundProxyName is invoked with below parameters

  • contextId: STATIC_INSTANCE
  • typeInfo: org.jboss.weld.util.Proxies$TypeInfo@61a3134c
  • bean:
    Session bean [class com.sun.ts.tests.ejb30.lite.view.equals.StatelessEqualsBean with qualifiers [@Any @Default]; local interfaces are [StatelessEqualsBean, BusinessLocalIF1, BusinessLocalIF2, AnnotatedLocalBusinessInterface1]
  • name: StatelessEqualsBean$

The proxyPackage of the bean is set(incorrectly for this scenario) here in the first iteration.
with

  • typeInfo.getInterfaces =
    [interface com.sun.ts.tests.ejb30.common.busiface.AnnotatedLocalBusinessInterface1,
    interface com.sun.ts.tests.ejb30.common.busiface.BusinessLocalIF1,
    interface com.sun.ts.tests.ejb30.common.busiface.BusinessLocalIF2]
  • type = interface com.sun.ts.tests.ejb30.common.busiface.AnnotatedLocalBusinessInterface1
  • declaringClass (= type.getDeclaringClass) is null
  • proxyPackage = null

Hence this line sets the proxyPackage after obtaining the package name of AnnotatedLocalBusinessInterface1 from this map. From the logs I see that the map does have all the 3 interfaces and the bean class correctly populated, no issue with that.

So effectively the bean gets the proxyPackage of interface.

@manovotn
Copy link
Contributor

Yes, I am aware of all that. However, for some other cases, this behavior as desired as described in https://issues.redhat.com/browse/WELD-2666 (and the linked PR) which introduced this bit of code.

So effectively the bean gets the proxyPackage of interface.

This isn't necessarily wrong, note that if you define classes with plain ClassLoader, this combination of package and class is perfectly valid.

I am trying to figure out a way to change the package of this proxy only for this particular case - so far my investigation indicates that this happens purely for EJB beans declaring a @Local interface they don't implement.
However, note that even if we decide to fix this, it would be for Weld 5 and 6. Weld 3 is no longer actively developed.

@manovotn
Copy link
Contributor

FYI I have created a PR draft that attempts to fix this - weld/core#2895
I am still not completely sure it won't interfere elsewhere, but so far it seems like an isolated case for EJB beans with @Local.
The test I added is indirect, as I am unable to verify class defining due to the reasons I mentioned above, but still verifies that the proxy now has package of the bean impl and not the interface.

@alwin-joseph
Copy link
Contributor Author

I see the PR with fix is merged. Thanks a lot for working on this.

I was looking at the prospects of extending this fix to WELD versions 3 & 4 (weld-core-impl jar).

As you clarified earlier,

However, note that even if we decide to fix this, it would be for Weld 5 and 6. Weld 3 is no longer actively developed.

are there any conditions which if satisfied can trigger updates in non-active versions(3&4).

In other option, should we be mindful of anything If we decide to fork the weld/core repo and apply the fix in the PR. I was thinking to do as below in the order

  • fork weld/core
  • apply the fix to impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java as in the PR
  • replace all 3.1.10-SNAPSHOT with 3.1.10
  • mvn clean install (from root or /impl)
  • Use impl/target/weld-core-impl-3.1.10.jar in our server implementation.

@manovotn
Copy link
Contributor

manovotn commented Oct 23, 2023

Note that you can still implement your own ProxyServices to bypass this entirely. That isn't an option for you?

are there any conditions which if satisfied can trigger updates in non-active versions(3&4).

There aren't any formal conditions for this. Jakarta EE 8 is now 4 years ago and JDK 11 is 5y.
TBH I am fairly surprised weblogic hasn't encountered this earlier - those EJB tests don't look like anything that's been tempered with recently.

In other option, should we be mindful of anything If we decide to fork the weld/core repo and apply the fix in the PR. I was thinking to do as below in the order

Yes, you could do that.
Probably compare the ProxyFactory file with what's in main branch now to be sure there aren't any other side effects, but tests should tell you if that's the case.

@alwin-joseph
Copy link
Contributor Author

Note that you can still implement your own ProxyServices to bypass this entirely. That isn't an option for you?

It was suggested not to use this approach for this release.

are there any conditions which if satisfied can trigger updates in non-active versions(3&4).

There aren't any formal conditions for this. Jakarta EE 8 is now 4 years ago and JDK 11 is 5y. TBH I am fairly surprised weblogic hasn't encountered this earlier - those EJB tests don't look like anything that's been tempered with recently.

I think this issue might have started after introducing impl/src/main/java11/org/jboss/weld/bean/proxy/util/WeldDefaultProxyServices.java 2 years back in 3.1 ; weblogic would have been using an older version of WELD before that.

In other option, should we be mindful of anything If we decide to fork the weld/core repo and apply the fix in the PR. I was thinking to do as below in the order

Yes, you could do that. Probably compare the ProxyFactory file with what's in main branch now to be sure there aren't any other side effects, but tests should tell you if that's the case.

Thanks! We will do this.

@alwin-joseph
Copy link
Contributor Author

Closing this as the fix is provided in weld. Thanks much @manovotn! Appreciate your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants