Cayenne
  1. Cayenne
  2. CAY-1487

Access to ObjectStore.objectMap not thread safe when creating new object instance (and processing snaphot events)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0 branch
    • Fix Version/s: 2.0.5
    • Component/s: Core Library
    • Labels:
      None

      Description

      The method ObjectStore.registerNode(..) is not synchronized, while getNode(..) is. The objectMap can be accessed in a thread-unsafe way when a new object instance is created and when at the same time snapshotevents are processed.

      When creating the instance, the unsafe registerNode method is used. It will start looping in the objectMap when at the same time when an EventDispatch thread tries to acces the objectMap through getNode(). The dispatch threads will go in a WAIT state which eventually results in a completely frozen application.

      I noticed that in cayenne 3 the registerNode() method is synchronized. Can such a fix also be applied to a cayenne 2.0.5 release?

        Activity

        Hide
        Andrus Adamchik added a comment -

        I committed synchronization code to 2.0 branch. Appreciate if you could build it an give it a try.

        Show
        Andrus Adamchik added a comment - I committed synchronization code to 2.0 branch. Appreciate if you could build it an give it a try.
        Hide
        Donald Vrakking added a comment -

        I don't see us upgrading to 3.x on the short term. It will require some (re)engineering effort.

        Show
        Donald Vrakking added a comment - I don't see us upgrading to 3.x on the short term. It will require some (re)engineering effort.
        Hide
        Andrus Adamchik added a comment -

        > The most obvious question; do you have an indication on when you have a fix ready?

        Not yet.

        > Or is there something we can try? In cayenne 3 the method ObjectStore.registerNode is synchronized. Is that all that need to be done or are there other considerations?

        Can you upgrade to 3.0.1?

        Show
        Andrus Adamchik added a comment - > The most obvious question; do you have an indication on when you have a fix ready? Not yet. > Or is there something we can try? In cayenne 3 the method ObjectStore.registerNode is synchronized. Is that all that need to be done or are there other considerations? Can you upgrade to 3.0.1?
        Hide
        Donald Vrakking added a comment -

        The most obvious question; do you have an indication on when you have a fix ready?

        Or is there something we can try? In cayenne 3 the method ObjectStore.registerNode is synchronized. Is that all that need to be done or are there other considerations?

        Show
        Donald Vrakking added a comment - The most obvious question; do you have an indication on when you have a fix ready? Or is there something we can try? In cayenne 3 the method ObjectStore.registerNode is synchronized. Is that all that need to be done or are there other considerations?
        Hide
        Andrus Adamchik added a comment -

        Thanks for the details. We'll fix it.

        Show
        Andrus Adamchik added a comment - Thanks for the details. We'll fix it.
        Hide
        Donald Vrakking added a comment -

        It occurs when the app is running in production. These are the traces taken using a profiler:

        TP-Processor17 [RUNNABLE] CPU time: 50:37
        java.util.HashMap.put(Object, Object)
        org.apache.cayenne.access.ObjectStore.registerNode(Object, Object)
        org.apache.cayenne.access.ObjectStore.recordObjectCreated(Persistent)
        org.apache.cayenne.access.DataContext.createAndRegisterNewObject(String)
        org.apache.cayenne.access.DataContext.createAndRegisterNewObject(Class)

        EventDispatchThread-3 [RUNNABLE] CPU time: 49:02
        java.util.HashMap.get(Object)
        org.apache.cayenne.access.ObjectStore.getNode(Object)
        org.apache.cayenne.access.ObjectStore.processInvalidatedIDs(Collection)
        org.apache.cayenne.access.ObjectStore.processSnapshotEvent(SnapshotEvent)
        org.apache.cayenne.access.ObjectStore.snapshotsChanged(SnapshotEvent)
        sun.reflect.GeneratedMethodAccessor6247.invoke(Object, Object[])
        sun.reflect.DelegatingMethodAccessorImpl.invoke(Object, Object[])
        java.lang.reflect.Method.invoke(Object, Object[])
        org.apache.cayenne.util.Invocation.fire(Object[])
        org.apache.cayenne.event.EventManager$InvocationDispatch.fire()
        org.apache.cayenne.event.EventManager$DispatchThread.run()

        The callstack from the first trace isn't synchronized at any point. Accessing the HashMap will cause both threads to spin forever. The other eventDispatchThreads will wait forever because the processSnapshotEvent is synchronized.

        Show
        Donald Vrakking added a comment - It occurs when the app is running in production. These are the traces taken using a profiler: TP-Processor17 [RUNNABLE] CPU time: 50:37 java.util.HashMap.put(Object, Object) org.apache.cayenne.access.ObjectStore.registerNode(Object, Object) org.apache.cayenne.access.ObjectStore.recordObjectCreated(Persistent) org.apache.cayenne.access.DataContext.createAndRegisterNewObject(String) org.apache.cayenne.access.DataContext.createAndRegisterNewObject(Class) EventDispatchThread-3 [RUNNABLE] CPU time: 49:02 java.util.HashMap.get(Object) org.apache.cayenne.access.ObjectStore.getNode(Object) org.apache.cayenne.access.ObjectStore.processInvalidatedIDs(Collection) org.apache.cayenne.access.ObjectStore.processSnapshotEvent(SnapshotEvent) org.apache.cayenne.access.ObjectStore.snapshotsChanged(SnapshotEvent) sun.reflect.GeneratedMethodAccessor6247.invoke(Object, Object[]) sun.reflect.DelegatingMethodAccessorImpl.invoke(Object, Object[]) java.lang.reflect.Method.invoke(Object, Object[]) org.apache.cayenne.util.Invocation.fire(Object[]) org.apache.cayenne.event.EventManager$InvocationDispatch.fire() org.apache.cayenne.event.EventManager$DispatchThread.run() The callstack from the first trace isn't synchronized at any point. Accessing the HashMap will cause both threads to spin forever. The other eventDispatchThreads will wait forever because the processSnapshotEvent is synchronized.
        Hide
        Andrus Adamchik added a comment -

        Sorry, was going to follow up on this, but it fell through the cracks... Are you actually seeing a problem with this code (map corruption), or did you find it via a code review? I am asking cause a quick check of the 2.0 code shows that invocations of 'registerNode' have external synchronization by the caller. E.g. in DataContext:

        synchronized (getGraphManager())

        { localObject = (Persistent) descriptor.createObject(); localObject.setObjectContext(this); localObject.setObjectId(id); getGraphManager().registerNode(id, localObject); }

        So while this may not be intuitive, it is not actually going to cause problems (as far as I can tell).

        Show
        Andrus Adamchik added a comment - Sorry, was going to follow up on this, but it fell through the cracks... Are you actually seeing a problem with this code (map corruption), or did you find it via a code review? I am asking cause a quick check of the 2.0 code shows that invocations of 'registerNode' have external synchronization by the caller. E.g. in DataContext: synchronized (getGraphManager()) { localObject = (Persistent) descriptor.createObject(); localObject.setObjectContext(this); localObject.setObjectId(id); getGraphManager().registerNode(id, localObject); } So while this may not be intuitive, it is not actually going to cause problems (as far as I can tell).
        Hide
        Donald Vrakking added a comment -

        Is there any followup on this?

        Show
        Donald Vrakking added a comment - Is there any followup on this?

          People

          • Assignee:
            Unassigned
            Reporter:
            Donald Vrakking
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development