Jetty
  1. Jetty
  2. JETTY-529

ClassNotFoundException when deserializing Array from session

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.1.7, 6.1.8
    • Fix Version/s: 7.0.0pre1, 6.1.10
    • Component/s: Servlet
    • Labels:
      None
    • Environment:
      Eclipse Equinox OSGi on Linux (32 bit)
    • Number of attachments :
      0

      Description

      HashSessionManager.resolveClass fails to restore sessions that contain arrays of objects loaded from the webcontext where it fails with a ClassNotFoundException.

      This appears to be due to use of ClassLoader.loadClass which is not always able to decode serialization array syntax (e.g. "[Ljava.lang.Object"). There is a good description of the problem and solution here: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500212

      I patched my Jetty locally to change the following line in HashSessionManager.ClassLoadingObjectInputStream.resolveClass:586
      return Thread.currentThread().getContextClassLoader().loadClass(cl.getName());
      to the following (as recommended by the bug link above):
      return Class.forName(cl.getName(), false, Thread.currentThread().getContextClassLoader());
      and the deserialization now works for me.

      This issue manifests in Jdk 1.5 & 1.6 and is resolved by the suggested fix.

        Activity

        Hide
        Greg Wilkins added a comment -

        as you are interested in clustering at the moment

        Show
        Greg Wilkins added a comment - as you are interested in clustering at the moment
        Hide
        Simon Kaegi added a comment -

        You might want to be careful here about using Class.forName along with the context class loader especially in an OSGi environment. This method has the unfortunate characteristic of pinning (in the hidden VM cache) any loaded classes to the context class loader. Also see http://www.eclipsecon.org/2008/index.php?page=sub/&id=377

        Show
        Simon Kaegi added a comment - You might want to be careful here about using Class.forName along with the context class loader especially in an OSGi environment. This method has the unfortunate characteristic of pinning (in the hidden VM cache) any loaded classes to the context class loader. Also see http://www.eclipsecon.org/2008/index.php?page=sub/&id=377
        Hide
        Dave Wallace added a comment -

        Thanks for pointing this out, Simon. It's an interesting issue and I'm not a ClassLoader (or Jetty) expert but I think class pinning will not be a problem here. In this case, the ThreadContextClassLoader will always be set by Jetty to the configured WebAppClassLoader for the current web application context. This loader should be freed when the context is shutdown or refreshed and a new one created when the application is started.

        Hargrave's blog post on the subject at http://blog.bjhargrave.com/2007/09/classforname-caches-defined-class-in.html is a bit more revealing of the issues. I think he is primarily concerned with Eclipse Equinox's ContextFinder classloader implementation which apparently changes its delegation chain on the fly. I don't think ContextFinder is needed here since Jetty has a nicely-designed IOC-friendly API. I had little trouble injecting a custom BundleClassLoader as the parent of Jetty's WebAppClassLoader. Using OSGi's service layer, Jetty webapp life-cycle is tied to the requesting bundle's life-cycle as a matter of course.

        In any case, the SDN bug I linked in this bug's description is pretty unequivocal about ClassLoader.loadClass not being intended to handle array syntax. So, OSGi or not, if Class.forName does turn out to be inappropriate something special will be needed for loading arrays.

        Show
        Dave Wallace added a comment - Thanks for pointing this out, Simon. It's an interesting issue and I'm not a ClassLoader (or Jetty) expert but I think class pinning will not be a problem here. In this case, the ThreadContextClassLoader will always be set by Jetty to the configured WebAppClassLoader for the current web application context. This loader should be freed when the context is shutdown or refreshed and a new one created when the application is started. Hargrave's blog post on the subject at http://blog.bjhargrave.com/2007/09/classforname-caches-defined-class-in.html is a bit more revealing of the issues. I think he is primarily concerned with Eclipse Equinox's ContextFinder classloader implementation which apparently changes its delegation chain on the fly. I don't think ContextFinder is needed here since Jetty has a nicely-designed IOC-friendly API. I had little trouble injecting a custom BundleClassLoader as the parent of Jetty's WebAppClassLoader. Using OSGi's service layer, Jetty webapp life-cycle is tied to the requesting bundle's life-cycle as a matter of course. In any case, the SDN bug I linked in this bug's description is pretty unequivocal about ClassLoader.loadClass not being intended to handle array syntax. So, OSGi or not, if Class.forName does turn out to be inappropriate something special will be needed for loading arrays.
        Hide
        Jesse McConnell added a comment -

        this is a messy issue...

        I will look into that patch and try and get an appropriate test case setup for it as well, thanks for the comments as it flagged the issue for me

        cheers

        Show
        Jesse McConnell added a comment - this is a messy issue... I will look into that patch and try and get an appropriate test case setup for it as well, thanks for the comments as it flagged the issue for me cheers
        Hide
        Jesse McConnell added a comment -

        applied the fix to both trunk and the 6.1 branch

        thanks for linking the sun resources regarding the issue and analysis.

        cheers!

        Show
        Jesse McConnell added a comment - applied the fix to both trunk and the 6.1 branch thanks for linking the sun resources regarding the issue and analysis. cheers!

          People

          • Assignee:
            Jesse McConnell
            Reporter:
            Dave Wallace
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: