Bug 960002 - Regression: Nested class within an invalid class causes deployment failure
Summary: Regression: Nested class within an invalid class causes deployment failure
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: JBoss Enterprise Application Platform 6
Classification: JBoss
Component: CDI/Weld
Version: 6.1.0
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ER7
: EAP 6.1.0
Assignee: David M. Lloyd
QA Contact: Ron Šmeral
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-06 12:02 UTC by Ron Šmeral
Modified: 2016-11-01 01:37 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-07-23 18:34:51 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Reproducer (2.95 KB, application/x-java-archive)
2013-05-06 12:02 UTC, Ron Šmeral
no flags Details
ER6-Weld (63.22 KB, text/x-log)
2013-05-07 13:42 UTC, Ron Šmeral
no flags Details
ER6-Seam-chatroom-log (106.55 KB, text/x-log)
2013-05-07 13:49 UTC, Ron Šmeral
no flags Details
Seam Chatroom reproducer (1.84 MB, application/octet-stream)
2013-05-07 14:06 UTC, Ron Šmeral
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 961249 0 unspecified CLOSED Seam 2.3, ClassDescriptor doesn't catch LinkageError 2021-02-22 00:41:40 UTC
Red Hat Bugzilla 961470 0 unspecified CLOSED Regression: CDI portable extensions with missing dependencies fail to deploy 2021-02-22 00:41:40 UTC
Red Hat Issue Tracker WELD-1215 0 None None None Never

Internal Links: 961249 961470

Description Ron Šmeral 2013-05-06 12:02:07 UTC
Created attachment 744092 [details]
Reproducer

Description of problem:
The Weld test org.jboss.weld.tests.beanDeployment.noclassdeffound.NoClassDefFoundErrorTest
fails on EAP 6.1.0.ER6.
The same problem was already fixed in Weld in https://issues.jboss.org/browse/WELD-1215, so it seems that it is now caused by a change in class loading/JBoss Modules.

org.jboss.arquillian.container.spi.client.container.DeploymentException: Cannot deploy: 0e0d263b-72a0-49ca-9950-73d6403a679d.jar
	at org.jboss.as.arquillian.container.ArchiveDeployer.deployInternal(ArchiveDeployer.java:83)
	at org.jboss.as.arquillian.container.ArchiveDeployer.deployInternal(ArchiveDeployer.java:64)
	at org.jboss.as.arquillian.container.ArchiveDeployer.deploy(ArchiveDeployer.java:46)
...
Caused by: java.lang.Exception: {"JBAS014671: Failed services" => {"jboss.deployment.unit.\"0e0d263b-72a0-49ca-9950-73d6403a679d.jar\".WeldStartService" => "org.jboss.msc.service.StartException in service jboss.deployment.unit.\"0e0d263b-72a0-49ca-9950-73d6403a679d.jar\".WeldStartService: Failed to start service
    Caused by: java.lang.LinkageError: Failed to link org/jboss/weld/tests/beanDeployment/noclassdeffound/Bar (Module \"deployment.0e0d263b-72a0-49ca-9950-73d6403a679d.jar:main\" from Service Module Loader)
    Caused by: java.lang.NoClassDefFoundError: org.jboss.weld.tests.beanDeployment.noclassdeffound.Foo
    Caused by: java.lang.ClassNotFoundException: org.jboss.weld.tests.beanDeployment.noclassdeffound.Foo from [Module \"deployment.0e0d263b-72a0-49ca-9950-73d6403a679d.jar:main\" from Service Module Loader]"}}
	at org.jboss.as.controller.client.helpers.standalone.impl.ServerDeploymentPlanResultFuture.getActionResult(ServerDeploymentPlanResultFuture.java:134)
...

Version-Release number of selected component (if applicable):
EAP 6.1.0.ER6
Weld 1.1.12.Final-redhat-1

How reproducible:
Deploy the attached reproducer to EAP.
Alternatively, run the org.jboss.weld.tests.beanDeployment.noclassdeffound.NoClassDefFoundErrorTest

Actual results:
Archive does not deploy, DeploymentException is thrown.

Expected results:
The NCDFE is caught, the invalid class is not regarded as a bean,  and the archive is succesfully deployed.

Comment 1 Marek Schmidt 2013-05-06 17:13:09 UTC
Note that this used to work on EAP 6.1.0.ER5, so it is a very recent regression.

I can also reproduce the problem on some Seam 2.3 examples, where seam-remoting contains org/jboss/seam/remoting/gwt/GWTService, which implements com.google.gwt.user.server.rpc.SerializationPolicyProvider, which is contained an optional gwt-servlet dependency. This example deploys on EAP6 <= 6.1.0.ER5 with a warning ( Failed to define class org.jboss.seam.remoting.gwt.GWTService  ), but fails to deploy on EAP 6.1.0.ER6. (it works if I add the missing gwt-servlet jar)

Comment 2 Dimitris Andreadis 2013-05-07 09:05:27 UTC
How common is this problem? I mean, what is the expected behavior, since the outclass is already invalid, shouldn't the deployment fail, anyway?

David, can you comment?

Comment 3 Marek Schmidt 2013-05-07 10:08:04 UTC
Well, on all the previous versions of EAP6, such deployment would produce a warning, such as 

12:02:23,135 WARN  [org.jboss.modules] (MSC service thread 1-2) Failed to define class org.jboss.weld.tests.beanDeployment.noclassdeffound.Bar in Module "deployment.NoClassDefFoundErrorTest.jar:main" from Service 
Module Loader: java.lang.LinkageError: Failed to link org/jboss/weld/tests/beanDeployment/noclassdeffound/Bar (Module "deployment.NoClassDefFoundErrorTest.jar:main" from Service Module Loader)

but would continue with the deployment. e.g. Weld explicitly checks for this condition and will produce an additional INFO message:

WELD-000119 Not generating any bean definitions from org.jboss.weld.tests.beanDeployment.noclassdeffound.Bar$ThisWillCauseAnError because of underlying class loading error

so I assume this has been a feature that such deployments are allowed (with a warning). The use case could be an library with some optional dependencies, that should continue to work when those optional dependencies are not provided (as is the case of Seam Remoting and its GWT servlet dependency).

Comment 5 Ron Šmeral 2013-05-07 13:26:27 UTC
This commit https://github.com/jbossas/jboss-modules/commit/29015b4be5f015f02978e06a96ceca0781cbe6c4 is the culprit. 
I tried reverting it and it fixes the problem.

Comment 6 David M. Lloyd 2013-05-07 13:29:09 UTC
Unfortunately you are wrong.  The commit changes the exception that is thrown; it is not the cause of the problem.  The problem is that the code which catches these exceptions in the Weld deployment layer does not catch all the exceptions that it should; this commit just adds a circumstance in which that problem is triggered.

Would it be possible to get the complete stack trace for the exception?

Comment 7 Ron Šmeral 2013-05-07 13:42:05 UTC
Created attachment 744738 [details]
ER6-Weld

Run of NoClassDefFoundErrorTest
First run - unmodified ER6
Second run - ER6 with jboss-modules commit 29015b4b reverted

Line 56 - first warn
Line 172,215 - error

Line 321 - second run

Comment 8 Ron Šmeral 2013-05-07 13:49:32 UTC
Created attachment 744739 [details]
ER6-Seam-chatroom-log

Deployment of Seam Chatroom example (uses remoting, has dependency on gwt-servlet)

First run - unmodified ER6
Second run - ER6 with jboss-modules commit 29015b4b reverted

L177 - first warn
L392 - error

L447 - second run

Comment 9 Marek Schmidt 2013-05-07 13:56:45 UTC
If that's the case, this would have to be also fixed in JBoss Seam integration.

Comment 10 Ron Šmeral 2013-05-07 14:06:57 UTC
Created attachment 744754 [details]
Seam Chatroom reproducer

Comment 11 David M. Lloyd 2013-05-07 14:18:34 UTC
The problem is that BeanDeployer inside of Weld does not robustly handle linkage problems.  The code:

    private boolean isBeanCandidate(Class<?> clazz) {
        try {
            return !clazz.isAnnotation() && !clazz.isEnum() && !Reflections.isNonStaticInnerClass(clazz);
        } catch (NoClassDefFoundError e) {
            logIgnoredClass(clazz.getName(), e);
            return false;
        }
    }

should be:

    private boolean isBeanCandidate(Class<?> clazz) {
        try {
            return !clazz.isAnnotation() && !clazz.isEnum() && !Reflections.isNonStaticInnerClass(clazz);
        } catch (LinkageError e) {
            logIgnoredClass(clazz.getName(), e);
            return false;
        }
    }

I'll submit a pull request to the Weld project to get this fixed.

Comment 13 David M. Lloyd 2013-05-07 14:25:06 UTC
https://github.com/weld/core/pull/303 has the fix for this issue.

Comment 14 Marek Schmidt 2013-05-07 14:33:41 UTC
The fix wouldn't fix the Seam case though, would it?

Comment 15 David M. Lloyd 2013-05-07 14:38:59 UTC
No, it definitely will not.  I do not know the Seam code base so I cannot find the error, but odds are very good it is a similar problem.

Comment 17 Paul Gier 2013-05-07 19:06:47 UTC
I applied the weld patch to the product build of weld 1.1.12.Final and rebuilt it for ER7.
https://brewweb.devel.redhat.com/buildinfo?buildID=270641

Comment 18 Stuart Douglas 2013-05-08 00:51:59 UTC
I have released Weld 1.1.13 with David's fix, and I have also identified and fixed a few other places in the Weld code base that were subject to similar problems. 

PR is here: https://github.com/jbossas/jboss-eap/pull/143

Comment 19 Dimitris Andreadis 2013-05-08 07:19:14 UTC
Right, Stuart's new Weld release fixes the bug in multiple locations, so we should be using that instead!

Comment 21 Ron Šmeral 2013-05-09 14:43:18 UTC
Verified on EAP 6.1.0.ER7. The Weld problem is fixed.

Comment 22 Ron Šmeral 2013-05-09 17:17:42 UTC
Even though this particular problem is fixed, there is another problem -- if we deploy a CDI portable extension with missing dependencies, the error is back -- #961470.

I have a question in this regard: what was the reason for the change in exception/error handling (https://github.com/jbossas/jboss-modules/commit/29015b4b) in the JBoss Modules classloader? Should LinkageError be considered "unexpected" in ModuleClassLoader? (honestly interested)

Comment 23 David M. Lloyd 2013-05-09 17:52:50 UTC
The reason for the change in error handing is that when the exception is wrapped, the JDK tends to trim off the cause, resulting in useless stack traces and hard-to-diagnose problems.  It also causes an OSGi TCK problem, and a few other miscellaneous issues related to the fact that the specific exception type is wrong so it causes the wrong action to be taken (this is especially true around class definition, which we do concurrently and which needs the right error type to be thrown to work correctly).

LinkageError is the base class for run time class loading issues.  NoClassDefFoundError is a subclass of LinkageError.  What happens is, people coded for the specific exception they *saw* "that one time" without understanding the reason for the exception and the other similar possible problems that can occur.  LinkageError is always the right thing to catch when you want to "absorb" a runtime class load problem.  Even were that patch reverted and the other problems ignored, there would still be common failure scenarios in that code which would just immediately come back to haunt us as soon as someone hits them.  This includes, wrong bytecode version errors, verify errors, missing dependencies on the immediate class (rather than a transitive class as is the case here).


Note You need to log in before you can comment on or make changes to this bug.