Classloader issues under JBoss 6.X - 7.X - Patch Included

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Classloader issues under JBoss 6.X - 7.X - Patch Included

larzeni
Hi to all,
few years ago, I raised an alert concerning the compatibility between Tapestry5 and JBoss; the thread was:

"Is tapestry plastic incompatible with JEE specs?"

https://mail-archives.apache.org/mod_mbox/tapestry-users/201512.mbox/%3Ctrinity-fc3b0e36-0402-4d71-b48a-4a464cbd6474-1449857158334@3capp-mailcom-lxa14%3E

At that time I found a workaround and concluded that there was no way to solve the issue, thinking that was a problem related to the jboss classloader (or, more generally, between an appserver classloader and the behaviour of plastic).

Few days ago, working on a new project under JBoss 7.0.0.GA, I was again badly hitted by this problem, but this time I investigated more thoroughly the issue and I found the source of all evil: the class "org.apache.tapestry5.internal.plastic.asm.ClassWriter" of plastic.

The classloader used to find (and load) the classes by plastic is really not compatible with an appserver. An appserver is required to enforce classloaded isolation and plastic ignores this.
Anyway plastic can be patched to solve the issue; here I attach a patch, please I ask to some developer to kindly forward and apply this patch to the 5.4.x branch and to the 5.5.

I also want to give thanks to these guys to help me to focus the problem:

https://stackoverflow.com/questions/42423907/unable-to-move-tapestry-jars-out-of-the-war-with-page-code-inside-the-war-still
https://blog.progs.be/50/tapestry-classloading-problems-on-jboss

Regards,
Luca Arzeni


*** Here is the original code: ***

1752     protected String getCommonSuperClass(final String type1, final String type2) {
1753         Class<?> c, d;
1754         ClassLoader classLoader = getClass().getClassLoader();
1755         try {
1756             c = Class.forName(type1.replace('/', '.'), false, classLoader);
1757             d = Class.forName(type2.replace('/', '.'), false, classLoader);
1758         } catch (Exception e) {
1759             throw new RuntimeException(e.toString());
1760         }
1761         if (c.isAssignableFrom(d)) {
1762             return type1;
1763         }
1764         if (d.isAssignableFrom(c)) {
1765             return type2;
1766         }
1767         if (c.isInterface() || d.isInterface()) {
1768             return "java/lang/Object";
1769         } else {
1770             do {
1771                 c = c.getSuperclass();
1772             } while (!c.isAssignableFrom(d));
1773             return c.getName().replace('.', '/');
1774         }
1775     }

*** Here is the patch: ***

1752    protected String getCommonSuperClass(final String type1, final String type2) {
1753        Class<?> c, d;
1754        ClassLoader classLoader = getClass().getClassLoader();
1755        try {
1756            c = Class.forName(type1.replace('/', '.'), false, classLoader);
1757            d = Class.forName(type2.replace('/', '.'), false, classLoader);
1758        } catch (Exception e) {
// --- ARZILLO PATCH BEGIN -----------------------------------------------------------

    System.err.println("WARNING: type1:" + type1 + ", type2: " + type2 + ", exception: " + e + ", classLoader: " + classLoader +", attempting to use the classloader of the current thread");

    classLoader = Thread.currentThread().getContextClassLoader();
        if ( classLoader != null ) {

        try {
        c = Class.forName(type1.replace('/', '.'), false, classLoader);
        d = Class.forName(type2.replace('/', '.'), false, classLoader);
        }
                catch (Exception e_inner) {
            System.err.println("ERROR 1: type1:" + type1 + ", type2: " + type2 + ", e_inner: " + e_inner + ", classLoader: " + classLoader +", failed even using the classloader of the current thread");
            throw new RuntimeException(e_inner.toString());
                }
        }
        else {
        System.err.println("ERROR 2: type1:" + type1 + ", type2: " + type2 + ", exception: " + e + ", classLoader: " + classLoader +", unable to get the classloader of the current thread");
        throw new RuntimeException(e.toString());
        }
// --- ARZILLO PATCH END -----------------------------------------------------------
1760        }
1761 if (c.isAssignableFrom(d)) {
1762            return type1;
1763        }
1764        if (d.isAssignableFrom(c)) {
1765            return type2;
1766        }
1767        if (c.isInterface() || d.isInterface()) {
1768            return "java/lang/Object";
1779        } else {
1770            do {
1771                c = c.getSuperclass();
1772            } while (!c.isAssignableFrom(d));
1773            return c.getName().replace('.', '/');
1774        }
1775    }



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Classloader issues under JBoss 6.X - 7.X - Patch Included

Marvin Monroe
Nice job!

I think the general path to get things patched is to open a Jira issue and attach a proper patch file, possibly including a test. The committers can’t use your code otherwise for apache license reasons IIRC…

Cheers
Christian


> Am 01.12.2017 um 17:38 schrieb Luca Arzeni <[hidden email]>:
>
> Hi to all,
> few years ago, I raised an alert concerning the compatibility between Tapestry5 and JBoss; the thread was:
>
> "Is tapestry plastic incompatible with JEE specs?"
>
> https://mail-archives.apache.org/mod_mbox/tapestry-users/201512.mbox/%3Ctrinity-fc3b0e36-0402-4d71-b48a-4a464cbd6474-1449857158334@3capp-mailcom-lxa14%3E
>
> At that time I found a workaround and concluded that there was no way to solve the issue, thinking that was a problem related to the jboss classloader (or, more generally, between an appserver classloader and the behaviour of plastic).
>
> Few days ago, working on a new project under JBoss 7.0.0.GA, I was again badly hitted by this problem, but this time I investigated more thoroughly the issue and I found the source of all evil: the class "org.apache.tapestry5.internal.plastic.asm.ClassWriter" of plastic.
>
> The classloader used to find (and load) the classes by plastic is really not compatible with an appserver. An appserver is required to enforce classloaded isolation and plastic ignores this.
> Anyway plastic can be patched to solve the issue; here I attach a patch, please I ask to some developer to kindly forward and apply this patch to the 5.4.x branch and to the 5.5.
>
> I also want to give thanks to these guys to help me to focus the problem:
>
> https://stackoverflow.com/questions/42423907/unable-to-move-tapestry-jars-out-of-the-war-with-page-code-inside-the-war-still
> https://blog.progs.be/50/tapestry-classloading-problems-on-jboss
>
> Regards,
> Luca Arzeni
>
>
> *** Here is the original code: ***
>
> 1752     protected String getCommonSuperClass(final String type1, final String type2) {
> 1753         Class<?> c, d;
> 1754         ClassLoader classLoader = getClass().getClassLoader();
> 1755         try {
> 1756             c = Class.forName(type1.replace('/', '.'), false, classLoader);
> 1757             d = Class.forName(type2.replace('/', '.'), false, classLoader);
> 1758         } catch (Exception e) {
> 1759             throw new RuntimeException(e.toString());
> 1760         }
> 1761         if (c.isAssignableFrom(d)) {
> 1762             return type1;
> 1763         }
> 1764         if (d.isAssignableFrom(c)) {
> 1765             return type2;
> 1766         }
> 1767         if (c.isInterface() || d.isInterface()) {
> 1768             return "java/lang/Object";
> 1769         } else {
> 1770             do {
> 1771                 c = c.getSuperclass();
> 1772             } while (!c.isAssignableFrom(d));
> 1773             return c.getName().replace('.', '/');
> 1774         }
> 1775     }
>
> *** Here is the patch: ***
>
> 1752    protected String getCommonSuperClass(final String type1, final String type2) {
> 1753        Class<?> c, d;
> 1754        ClassLoader classLoader = getClass().getClassLoader();
> 1755        try {
> 1756            c = Class.forName(type1.replace('/', '.'), false, classLoader);
> 1757            d = Class.forName(type2.replace('/', '.'), false, classLoader);
> 1758        } catch (Exception e) {
> // --- ARZILLO PATCH BEGIN -----------------------------------------------------------
>
>     System.err.println("WARNING: type1:" + type1 + ", type2: " + type2 + ", exception: " + e + ", classLoader: " + classLoader +", attempting to use the classloader of the current thread");
>
>     classLoader = Thread.currentThread().getContextClassLoader();
>         if ( classLoader != null ) {
>
>         try {
>         c = Class.forName(type1.replace('/', '.'), false, classLoader);
>         d = Class.forName(type2.replace('/', '.'), false, classLoader);
>         }
>                catch (Exception e_inner) {
>             System.err.println("ERROR 1: type1:" + type1 + ", type2: " + type2 + ", e_inner: " + e_inner + ", classLoader: " + classLoader +", failed even using the classloader of the current thread");
>             throw new RuntimeException(e_inner.toString());
>                }
>         }
>         else {
>         System.err.println("ERROR 2: type1:" + type1 + ", type2: " + type2 + ", exception: " + e + ", classLoader: " + classLoader +", unable to get the classloader of the current thread");
>         throw new RuntimeException(e.toString());
>         }
> // --- ARZILLO PATCH END -----------------------------------------------------------
> 1760        }
> 1761 if (c.isAssignableFrom(d)) {
> 1762            return type1;
> 1763        }
> 1764        if (d.isAssignableFrom(c)) {
> 1765            return type2;
> 1766        }
> 1767        if (c.isInterface() || d.isInterface()) {
> 1768            return "java/lang/Object";
> 1779        } else {
> 1770            do {
> 1771                c = c.getSuperclass();
> 1772            } while (!c.isAssignableFrom(d));
> 1773            return c.getName().replace('.', '/');
> 1774        }
> 1775    }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]