Uploaded image for project: 'James Server'
  1. James Server
  2. JAMES-418

Loader uses wrong method to obtain class loader/doesn't set context class loader

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.0
    • Fix Version/s: 2.3.0
    • Component/s: James Core
    • Labels:
      None

      Description

      I had difficulty loading resources from my classes directory. In reviewing the Loader source code, I see two problems:
      1) It uses this.class.getClassLoader(), rather than the preferred/standard Thread.currentThread.getContextClassLoader(). This is not a problem right now, as the Apache James developers have control over the entire application, but could become a difficult bug to track down in the future. In addition, as soon as you start adding multiple class loaders, chaining class loaders with parents becomes impossible (see next point) because it sets the same class loader as multiple parents. In the source code, all instances of this.getClass().getClassLoader() (or whatever.getClass().getClassLoader()) should be replaced by Thread.currentThread().getContextClassLoader()
      2) The greater problem is that it does not call Thread.currentThread().setContextClassLoader(classLoader). This means that any code that uses the standard method Thread.currentThread().getContextClassLoader() (as my code does) will not get the correct class loader, and thus will not be able to load the appropriate resources. In fact, I was getting the primary class loader, which only loads the Phoenix Jar. I had to add into my code the following (the class loader's parent is already set):
      Thread.currentThread().setContextClassLoader(MyClass.class.getClassLoader());
      By the nature of Java class loaders, it is expected that the thread's Context ClassLoader is always kept current, and that any new class loaders are added to the chain. I think that this change should be made in the Apache James source code.

      1. cornerstone-datasources-impl-2.1.jar
        9 kB
        Stefano Bagnara
      2. phoenix-trunk.zip
        2.13 MB
        Stefano Bagnara

        Activity

        Hide
        bago Stefano Bagnara added a comment -

        The whole classloading issue should be demanded to the container.
        In fact latest phoenix trunk contains code to automatically provide SAR-INF/lib and SAR-INF/classes support to the contained application classloader.

        We can provide a workaround for mailets but the problem should be fixed by using a better container.

        Unfortunately I've not been able to run james in newer phoenix (since phoenix 4.1.0alpha svn16333 to latest trunk)

        Show
        bago Stefano Bagnara added a comment - The whole classloading issue should be demanded to the container. In fact latest phoenix trunk contains code to automatically provide SAR-INF/lib and SAR-INF/classes support to the contained application classloader. We can provide a workaround for mailets but the problem should be fixed by using a better container. Unfortunately I've not been able to run james in newer phoenix (since phoenix 4.1.0alpha svn16333 to latest trunk)
        Hide
        bago Stefano Bagnara added a comment -

        I tryed Loom-1.0 RC3 (based on phoenix 4.1.0alpha) and actually after a few fixes in James it works correclty and we would not need to manually add SAR-INF/lib and SAR-INF/classes to our classloading because loom already does it for us.

        Loom also support hot deploy (don't know wether hot re-deploy is supported too).

        Show
        bago Stefano Bagnara added a comment - I tryed Loom-1.0 RC3 (based on phoenix 4.1.0alpha) and actually after a few fixes in James it works correclty and we would not need to manually add SAR-INF/lib and SAR-INF/classes to our classloading because loom already does it for us. Loom also support hot deploy (don't know wether hot re-deploy is supported too).
        Hide
        bago Stefano Bagnara added a comment -

        Latest phoenix trunk also correnctly handle classloaders for contained applications.

        Show
        bago Stefano Bagnara added a comment - Latest phoenix trunk also correnctly handle classloaders for contained applications.
        Hide
        bago Stefano Bagnara added a comment -

        Here are the phoenix-trunk provided by peter royal, upgraded to the latest cornerstone/excalibur and a patched cornerstone-datasources-impl to make it running.
        Without the patch it does not start due to validation errors in the database-connections of the config.xml

        Show
        bago Stefano Bagnara added a comment - Here are the phoenix-trunk provided by peter royal, upgraded to the latest cornerstone/excalibur and a patched cornerstone-datasources-impl to make it running. Without the patch it does not start due to validation errors in the database-connections of the config.xml
        Hide
        noel Noel J. Bergman added a comment -

        If Peter Royal will tag the sources in SVN, I'd be comfortable to put these into the test environment, but I don't want untagged code in our builds.

        Show
        noel Noel J. Bergman added a comment - If Peter Royal will tag the sources in SVN, I'd be comfortable to put these into the test environment, but I don't want untagged code in our builds.
        Hide
        bago Stefano Bagnara added a comment -

        Noel: we can refer the phoenix trunk by it's current version. A few month ago, while I was trying a tagged version of Loom, you asked help from Peter Royal about classloading issues and Loom vs Phoenix. The result of the thread was that Peter said that Loom is not used around while the phoenix-trunk is used in production in "many environment".

        I had no time to investigate on the validation issue I had with both loom and phoenix trunk until now. The "spaces in xml" issues verified on loom is not present in phoenix-trunk.

        I think we cannot expect someone to tag or release anything on the phoenix side (almost 2 years of inactivity).

        You suggested to talk with Peter, please talk with him again and try to make it clear what a "tag" will give to james.

        IMHO we need to upgrade the container because it's the only correct way to fix the classloading issues.

        Show
        bago Stefano Bagnara added a comment - Noel: we can refer the phoenix trunk by it's current version. A few month ago, while I was trying a tagged version of Loom, you asked help from Peter Royal about classloading issues and Loom vs Phoenix. The result of the thread was that Peter said that Loom is not used around while the phoenix-trunk is used in production in "many environment". I had no time to investigate on the validation issue I had with both loom and phoenix trunk until now. The "spaces in xml" issues verified on loom is not present in phoenix-trunk. I think we cannot expect someone to tag or release anything on the phoenix side (almost 2 years of inactivity). You suggested to talk with Peter, please talk with him again and try to make it clear what a "tag" will give to james. IMHO we need to upgrade the container because it's the only correct way to fix the classloading issues.
        Hide
        noel Noel J. Bergman added a comment -

        We spent years trying to get out from under an untagged version of Avalon that no one was quite sure what was in it. I do not want to see that again. An SVN tag is a trivial thing to do, and simply ensures that we have a clue as to what code we are running.

        I'll ping Peter Royal about it.

        Show
        noel Noel J. Bergman added a comment - We spent years trying to get out from under an untagged version of Avalon that no one was quite sure what was in it. I do not want to see that again. An SVN tag is a trivial thing to do, and simply ensures that we have a clue as to what code we are running. I'll ping Peter Royal about it.
        Hide
        bago Stefano Bagnara added a comment -

        Ok, but I think that in case Peter or anyone else will not apply the tag we should upgrade anyway, either by using the SVN revision or by committing the phoenix sources to the james repository for reference so we can tag them and we exactly know what is the distributed container.

        IMHO a tag on a repository that has not been touched in the last 2 years shouldn't be so important for our release cycle.

        We need to have a fixed 2.3.0 release and the current option is to upgrade to phoenix-trunk or to loom. Phoenix trunk (after a fix in the cornersone-datasources-impl) seems to work fine, loom has an incompatability interpreting the spaces in the config.xml file. Furthermore You and Peter, if I understood correctly, said that phoenix-trunk is preferred to loom. So if noone has a better option to fix this issue we should avoid further "show stoppers".

        Show
        bago Stefano Bagnara added a comment - Ok, but I think that in case Peter or anyone else will not apply the tag we should upgrade anyway, either by using the SVN revision or by committing the phoenix sources to the james repository for reference so we can tag them and we exactly know what is the distributed container. IMHO a tag on a repository that has not been touched in the last 2 years shouldn't be so important for our release cycle. We need to have a fixed 2.3.0 release and the current option is to upgrade to phoenix-trunk or to loom. Phoenix trunk (after a fix in the cornersone-datasources-impl) seems to work fine, loom has an incompatability interpreting the spaces in the config.xml file. Furthermore You and Peter, if I understood correctly, said that phoenix-trunk is preferred to loom. So if noone has a better option to fix this issue we should avoid further "show stoppers".
        Hide
        bago Stefano Bagnara added a comment -

        This should be fixed in the last trunk.
        Upgraded phoenix to 4.1trunk (4.2 ?) and removed the james specific code to change the classloaders for mailet/matchers.
        Make use of Thread.currentThread().getContextClassLoader().loadClass() to load matcher/mailet classes!

        Show
        bago Stefano Bagnara added a comment - This should be fixed in the last trunk. Upgraded phoenix to 4.1trunk (4.2 ?) and removed the james specific code to change the classloaders for mailet/matchers. Make use of Thread.currentThread().getContextClassLoader().loadClass() to load matcher/mailet classes!
        Hide
        bago Stefano Bagnara added a comment -

        test, the bug is "resolved" but still is in my unresolved view.. something weird in jira... let's try reopen and close it again!

        Show
        bago Stefano Bagnara added a comment - test, the bug is "resolved" but still is in my unresolved view.. something weird in jira... let's try reopen and close it again!
        Hide
        bago Stefano Bagnara added a comment -

        test, the bug is "resolved" but still is in my unresolved view.. something weird in jira... let's try reopen and close it again!

        Show
        bago Stefano Bagnara added a comment - test, the bug is "resolved" but still is in my unresolved view.. something weird in jira... let's try reopen and close it again!
        Hide
        danny@apache.org Danny Angus added a comment -

        Closing issue fixed in released version.

        Show
        danny@apache.org Danny Angus added a comment - Closing issue fixed in released version.

          People

          • Assignee:
            bago Stefano Bagnara
            Reporter:
            ben.lindahl Ben Lindahl
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development