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

Mix-ins vanish if registered before setDateFormat

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Cannot Reproduce
    • Affects Version/s: 1.8.1, 1.8.2, 1.8.3, 1.8.4, 1.8.5, 1.8.6, 1.9
    • Fix Version/s: None
    • Component/s: ObjectMapper
    • Labels:
      None
    • Environment:
      Linux OS;
      java version "1.6.0_26"
      Java(TM) SE Runtime Environment (build 1.6.0_26-b03)
      Java HotSpot(TM) 64-Bit Server VM (build 20.1-b02, mixed mode)
    • Number of attachments :
      0

      Description

      In some cases, when you call objectMapper.registerModule(module) before objectMapper.setDateFormat(dateFormat) the MapperConfig's protected HashMap<ClassKey,Class<?>> _mixInAnnotations gets mysteriously emptied. Note that this does not happen always, I have a unit test which just tests serialization and deserialization of an object and the test always passes, but in integration test environment this bug appears. However, if I first place objectMapper.setDateFormat(dateFormat) and then objectMapper.registerModule(module) things work nicely.

        Activity

        Hide
        Tatu Saloranta added a comment -

        Sounds like a problem with immutable (copy) constructors – thank you for reporting, I will figure out why this happens.

        Show
        Tatu Saloranta added a comment - Sounds like a problem with immutable (copy) constructors – thank you for reporting, I will figure out why this happens.
        Hide
        Tatu Saloranta added a comment -

        I can not see anything obviously wrong with the code, so I will need some help here.
        Can you share some code that exhibits this, maybe code that does registerModule() + setDateFormat(), along with initialization part of the module in question?

        Show
        Tatu Saloranta added a comment - I can not see anything obviously wrong with the code, so I will need some help here. Can you share some code that exhibits this, maybe code that does registerModule() + setDateFormat(), along with initialization part of the module in question?
        Hide
        Ivan Hristov added a comment -

        Hi Tatu,

        Here is the exact explanation and location of the bug: When we set the date format in ObjectMapper class line 960 (for version 1.8.1) we are calling the factory method DeserializationConfig.withDateFormat(DateFormat df) which is doing 'new DeserializationConfig(this, _base.withDateFormat(df));' which on the other hand is not copying the _mixInAnnotations of the source DeserializationConfig (the parameter src in the protected DeserializationConfig(DeserializationConfig src, MapperConfig.Base base) constructor).

        I would like to bring your attention to something I've noticed during my brief observation of the code: I've noticed that the DeserializationConfig is not thread-safe and using a non thread-safe object to construct a new object without any kind of synchronization is a bad idea. Of course this will may not bring havoc now but be prepared in the future.

        Cheers,
        Ivan

        Show
        Ivan Hristov added a comment - Hi Tatu, Here is the exact explanation and location of the bug: When we set the date format in ObjectMapper class line 960 (for version 1.8.1) we are calling the factory method DeserializationConfig.withDateFormat(DateFormat df) which is doing 'new DeserializationConfig(this, _base.withDateFormat(df));' which on the other hand is not copying the _mixInAnnotations of the source DeserializationConfig (the parameter src in the protected DeserializationConfig(DeserializationConfig src, MapperConfig.Base base) constructor). I would like to bring your attention to something I've noticed during my brief observation of the code: I've noticed that the DeserializationConfig is not thread-safe and using a non thread-safe object to construct a new object without any kind of synchronization is a bad idea. Of course this will may not bring havoc now but be prepared in the future. Cheers, Ivan
        Hide
        Tatu Saloranta added a comment -

        On thread-safety: notice that Jackson is NOT designed to allow concurrent changing of ObjectMapper configuration; rather guarantee is that iff one configures ObjectMapper completely first, then all further access is thread-safe ("config-first-then-use").
        This is why no explicit synchronization is added. Synchronization is properly handled for parts that are mutable and shared, namely, cached serializers and deserializers.
        However, to support some level of thread-safe re-configuration, ObjectReader and ObjectWriter objects were added, and to support them, we have tried to make all other configuration objects immutable (in functional style).
        They only allochanging of simple state, things that make sense to be changed on per-call basis.

        Now: I am looking at trunk (1.9.3), and as far as I can see it does in fact pass mix-in annotations properly.
        This is why I asked; the problem you point out sounds like exactly kind of thing that could cause this.
        I can have a look at 1.8.x branch, but since you indicated that this also affects 1.9 I wanted to start there.

        Show
        Tatu Saloranta added a comment - On thread-safety: notice that Jackson is NOT designed to allow concurrent changing of ObjectMapper configuration; rather guarantee is that iff one configures ObjectMapper completely first, then all further access is thread-safe ("config-first-then-use"). This is why no explicit synchronization is added. Synchronization is properly handled for parts that are mutable and shared, namely, cached serializers and deserializers. However, to support some level of thread-safe re-configuration, ObjectReader and ObjectWriter objects were added, and to support them, we have tried to make all other configuration objects immutable (in functional style). They only allochanging of simple state, things that make sense to be changed on per-call basis. Now: I am looking at trunk (1.9.3), and as far as I can see it does in fact pass mix-in annotations properly. This is why I asked; the problem you point out sounds like exactly kind of thing that could cause this. I can have a look at 1.8.x branch, but since you indicated that this also affects 1.9 I wanted to start there.
        Hide
        Ivan Hristov added a comment -

        I've indicated version 1.9, because the problem exists in version 1.9.2 and at the time of creating this issue only the tag 1.9 was available.

        The thread-safe point was meant only to bring your attention to the fact that there are no guarantees that someone won't change the configuration from multiple threads.

        Show
        Ivan Hristov added a comment - I've indicated version 1.9, because the problem exists in version 1.9.2 and at the time of creating this issue only the tag 1.9 was available. The thread-safe point was meant only to bring your attention to the fact that there are no guarantees that someone won't change the configuration from multiple threads.
        Hide
        Tatu Saloranta added a comment -

        Right (wrt mention), appreciated, and I am aware of that. Mix-in annotation part is actually one of last remaining things that I should rewrite to use an immutable variant of data structure.

        The thing is just this: 1.9 does not have specific problem you mentioned, unless I am mis-reading the code. So I am hoping to reproduce the issue (to have unit tests for regression); or if that is difficult, obviously try to find the reason.

        I am assuming that you are not trying to reconfigure ObjectMapper after initial config (i.e. after using it for data binding).

        Show
        Tatu Saloranta added a comment - Right (wrt mention), appreciated, and I am aware of that. Mix-in annotation part is actually one of last remaining things that I should rewrite to use an immutable variant of data structure. The thing is just this: 1.9 does not have specific problem you mentioned, unless I am mis-reading the code. So I am hoping to reproduce the issue (to have unit tests for regression); or if that is difficult, obviously try to find the reason. I am assuming that you are not trying to reconfigure ObjectMapper after initial config (i.e. after using it for data binding).
        Hide
        Tatu Saloranta added a comment -

        Since I unfortunately can not reproduce this issue currently, help would be appreciated. Even if just verifying that it still occurs with 1.9.3.

        Show
        Tatu Saloranta added a comment - Since I unfortunately can not reproduce this issue currently, help would be appreciated. Even if just verifying that it still occurs with 1.9.3.
        Hide
        Tatu Saloranta added a comment -

        Can not reproduce, closing.

        Show
        Tatu Saloranta added a comment - Can not reproduce, closing.

          People

          • Assignee:
            Tatu Saloranta
            Reporter:
            Ivan Hristov
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 3 hours
              3h
              Remaining:
              Remaining Estimate - 3 hours
              3h
              Logged:
              Time Spent - Not Specified
              Not Specified