Jackson JSON Processor
  1. Jackson JSON Processor
  2. JACKSON-802

Improve class loading to use both contextual ClassLoader and 'Class.forName(...)'

    Details

    • Type: New Feature New Feature
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.8.9, 1.9.6
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      1

      Description

      (note: inspired by JACKSON-350)

      Looks like classloading can be nasty problem (... never heard that one before? ).
      Since neither 'default' nor contextual class work perfectly, let's use try both; starting with contextual.

      If need be, can add more features in future. But let's first try simple improvement.

        Activity

        Hide
        Tatu Saloranta added a comment -

        Additionally should just add something like "DeserializationContext.findClass(String className)", to enclose details. This could also use fallbacks.

        Further, I think it actually makes sense to try both approaches: perhaps first contextual ClassLoader, and if that fails, Class.forName().

        Show
        Tatu Saloranta added a comment - Additionally should just add something like "DeserializationContext.findClass(String className)", to enclose details. This could also use fallbacks. Further, I think it actually makes sense to try both approaches: perhaps first contextual ClassLoader, and if that fails, Class.forName().
        Hide
        Tatu Saloranta added a comment -

        Implemented, now uses Class.forName() as fallback: let's see how this works.

        Show
        Tatu Saloranta added a comment - Implemented, now uses Class.forName() as fallback: let's see how this works.
        Hide
        Gaetan Pitteloud added a comment -

        (I'm on 1.9.10)
        The functionality is implemented in org.codehaus.jackson.map.util.ClassUtils.findClass(String), and used in ClassNameIdResolver and TypeParser, but you missed another direct usage of Class.forName(String), which is
        org.codehaus.jackson.map.deser.std.ClassDeserializer.deserialize(String).

        This actually led us to a problem when in an OSGi environment. In order to be able for Jackson to load our classes, we have set our classloader as the thread-context classloader before deserializion. This works fine except when we need to deserialize a String into Class<?> object, due to the fact that ClassDeserializer.deserialize(String) does not use thread-context classloader.

        Is there a reason why ClassDeserializer does not make a direct use of ClassUtils.findClass() ? (It would as well prevent you from duplicating the code related to primitives).

        Show
        Gaetan Pitteloud added a comment - (I'm on 1.9.10) The functionality is implemented in org.codehaus.jackson.map.util.ClassUtils.findClass(String), and used in ClassNameIdResolver and TypeParser, but you missed another direct usage of Class.forName(String), which is org.codehaus.jackson.map.deser.std.ClassDeserializer.deserialize(String). This actually led us to a problem when in an OSGi environment. In order to be able for Jackson to load our classes, we have set our classloader as the thread-context classloader before deserializion. This works fine except when we need to deserialize a String into Class<?> object, due to the fact that ClassDeserializer.deserialize(String) does not use thread-context classloader. Is there a reason why ClassDeserializer does not make a direct use of ClassUtils.findClass() ? (It would as well prevent you from duplicating the code related to primitives).
        Hide
        Gaetan Pitteloud added a comment -

        Proposed patch : ClassDeserializer that uses ClassUtil.findClass(String)

        Show
        Gaetan Pitteloud added a comment - Proposed patch : ClassDeserializer that uses ClassUtil.findClass(String)
        Hide
        Tatu Saloranta added a comment -

        Sounds good to me – will try to check this out later on when I have access to Codehaus SVN. I think it is simple oversight.

        Show
        Tatu Saloranta added a comment - Sounds good to me – will try to check this out later on when I have access to Codehaus SVN. I think it is simple oversight.
        Hide
        Tatu Saloranta added a comment -

        Fixed the remaining case; grepping didn't show up other cases. Thanks!

        This will be in 1.9.12 once it gets released.

        Show
        Tatu Saloranta added a comment - Fixed the remaining case; grepping didn't show up other cases. Thanks! This will be in 1.9.12 once it gets released.

          People

          • Assignee:
            Tatu Saloranta
            Reporter:
            Tatu Saloranta
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: