OpenJPA
  1. OpenJPA
  2. OPENJPA-1716

Deadlock with openjpa.Multithreaded=true

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0.0-M1, 2.0.0-M2, 2.0.0-M3, 2.0.0-beta, 2.0.0-beta2, 2.0.0-beta3, 2.0.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      There is a Deadlock by using Multithreaded=true and work with some Threads on the same Entitymanager.

      After many hours of debugging, i found a solution for that Deadlock: The Problem could be solved by adding another lock into the lock Method of the StateManagerImpl:
      StateManagerImpl ~line3308
      /**

      • Lock the state manager if the multithreaded option is set.
        */
        protected void lock()
        Unknown macro: { if (_instanceLock != null){ _broker.lock();//<- This is the new Part to fix the Deadlock _instanceLock.lock(); } }

      Worked for me without Problems for a long time.

      Some other Configurations:
      <persistence xmlns="http://java.sun.com/xml/ns/persistence"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0">
      <persistence-unit name="default" transaction-type="RESOURCE_LOCAL">
      <mapping-file>orm.xml</mapping-file>

      <class>...</class>
      <class>...</class>

      <properties>
      <property name="openjpa.OrphanedKeyAction" value="log(Channel=Orphans, Level=TRACE)" />

      <property name="openjpa.ConnectionFactoryProperties" value="PrettyPrint=true, PrettyPrintLineLength=80" />

      <property name="openjpa.Multithreaded" value="true" />
      <property name="openjpa.InverseManager" value="true" />
      <property name="openjpa.LockManager" value="none" />
      <property name="openjpa.WriteLockLevel" value="none" />
      <property name="openjpa.Compatibility" value="
      QuotedNumbersInQueries=true,
      CopyOnDetach=true,
      cascadeWithDetach=true,
      superclassDiscriminatorStrategyByDefault=false
      " />

      <property name="openjpa.DetachState" value="fetch-groups" />

      <property name="openjpa.jdbc.SynchronizeMappings" value="buildSchema" />

      <!-- Default DataSource -->
      <property name="openjpa.ConnectionURL" value="jdbc:h2:db/test" />
      <property name="openjpa.ConnectionDriverName" value="org.h2.Driver" />
      </properties>
      </persistence-unit>
      </persistence>

      1. deadlock-fix-OPENJPA-1716.patch.txt
        0.7 kB
        David Minor
      2. OPENJPA-1716.PATCH
        6 kB
        Rick Curtis

        Activity

        Hide
        Rick Curtis added a comment -

        David -
        > how I should go about making my application immune to these problems.
        Don't share Entities across threads that are associated with an active context. ie: Detach entities before sharing... I think that should do it.?

        Thanks,
        Rick

        Show
        Rick Curtis added a comment - David - > how I should go about making my application immune to these problems. Don't share Entities across threads that are associated with an active context. ie: Detach entities before sharing... I think that should do it.? Thanks, Rick
        Hide
        David Minor added a comment -

        Yes. From my understanding, the broker and the state manager used to share the same lock, but for performance reasons separate locks were introduced. The deadlock in this JIRA is the result of those locks being acquired in a different order depending on the call path.

        I'm a little fuzzy on where entity managers fit in, and how I should go about making my application immune to these problems. If I'm storing an entity in the user's session or in a cache somewhere, do I call the new detach() first so that cross-thread field access doesn't interact with the broker? Or is there some other thing I'm missing?

        Show
        David Minor added a comment - Yes. From my understanding, the broker and the state manager used to share the same lock, but for performance reasons separate locks were introduced. The deadlock in this JIRA is the result of those locks being acquired in a different order depending on the call path. I'm a little fuzzy on where entity managers fit in, and how I should go about making my application immune to these problems. If I'm storing an entity in the user's session or in a cache somewhere, do I call the new detach() first so that cross-thread field access doesn't interact with the broker? Or is there some other thing I'm missing?
        Hide
        Rick Curtis added a comment -

        Hey David, have you taken a read through OPENJPA-453? ... it does a pretty good job explaining the root issue here.

        Thanks,
        Rick

        Show
        Rick Curtis added a comment - Hey David, have you taken a read through OPENJPA-453 ? ... it does a pretty good job explaining the root issue here. Thanks, Rick
        David Minor made changes -
        Attachment deadlock-fix-OPENJPA-1716.patch.txt [ 12538061 ]
        Hide
        David Minor added a comment -

        I've attached the fix I'm using to avoid the deadlock I was seeing. It's similar to the original patch in that it locks the broker before the state manager, but only for the particular field access call where I was seeing deadlock.

        Show
        David Minor added a comment - I've attached the fix I'm using to avoid the deadlock I was seeing. It's similar to the original patch in that it locks the broker before the state manager, but only for the particular field access call where I was seeing deadlock.
        Hide
        Rick Curtis added a comment -

        > Should Multithreaded=true be dropped from OpenJPA then?
        I'd love to say yet, but we have to assume that this property is in use by some customers. Best case we could deprecate usage of the property.

        > Also, should I worry about fetch groups? They are non-standard and we rely upon them for performance.
        No, you have nothing to worry about with fetch groups. I think the real sticking point is that Multithreaded=true wasn't every implemented correctly and at this point no one has ponied up cycles for a complete fix.

        Show
        Rick Curtis added a comment - > Should Multithreaded=true be dropped from OpenJPA then? I'd love to say yet, but we have to assume that this property is in use by some customers. Best case we could deprecate usage of the property. > Also, should I worry about fetch groups? They are non-standard and we rely upon them for performance. No, you have nothing to worry about with fetch groups. I think the real sticking point is that Multithreaded=true wasn't every implemented correctly and at this point no one has ponied up cycles for a complete fix.
        Hide
        David Minor added a comment -

        Should Multithreaded=true be dropped from OpenJPA then? If this no longer works and is no longer going to be supported I think it should be made clear to those who have working applications based on 1.2.x who are trying to move to the latest version.

        Also, should I worry about fetch groups? They are non-standard and we rely upon them for performance.

        Show
        David Minor added a comment - Should Multithreaded=true be dropped from OpenJPA then? If this no longer works and is no longer going to be supported I think it should be made clear to those who have working applications based on 1.2.x who are trying to move to the latest version. Also, should I worry about fetch groups? They are non-standard and we rely upon them for performance.
        Hide
        Albert Lee added a comment -

        The JPA spec is very explicit on thread safety. I.e. EntityManagerFactory is thread-safe but EntityManager is NOT. There are many examples in the spec illustrate how to handle the condition using applicaton managed persistence context.

        I am not familar with the details of Apache Servicemix and Aries Blueprint. Is it a JEE compliant JPA container? If so, the injected contained-managed EntityManager should handle multi-thread call to the JEE component. The injected em is typically not the real entity manager but a proxy based on transaction context for handling context propagation in the JEE container. This feature is defined in the JPA spec.

        Application should not rely on JPA provider non-standard feature (multi-thread=true) to overcome application specific requirement. Otherwise the application will not be portable to other JPA provider and may be error-prone.

        Show
        Albert Lee added a comment - The JPA spec is very explicit on thread safety. I.e. EntityManagerFactory is thread-safe but EntityManager is NOT. There are many examples in the spec illustrate how to handle the condition using applicaton managed persistence context. I am not familar with the details of Apache Servicemix and Aries Blueprint. Is it a JEE compliant JPA container? If so, the injected contained-managed EntityManager should handle multi-thread call to the JEE component. The injected em is typically not the real entity manager but a proxy based on transaction context for handling context propagation in the JEE container. This feature is defined in the JPA spec. Application should not rely on JPA provider non-standard feature (multi-thread=true) to overcome application specific requirement. Otherwise the application will not be portable to other JPA provider and may be error-prone.
        Hide
        David Minor added a comment -

        I'm seeing this problem as well, and the second stack trace above is the same as mine.

        It seems the StateManagerImpl.beforeAccessField() method calls lock(), and subsequently _broker.isActive(), which causes the broker to lock. Is there a preferred order in which the locks should be obtained?

        Show
        David Minor added a comment - I'm seeing this problem as well, and the second stack trace above is the same as mine. It seems the StateManagerImpl.beforeAccessField() method calls lock(), and subsequently _broker.isActive(), which causes the broker to lock. Is there a preferred order in which the locks should be obtained?
        Hide
        Rick Curtis added a comment -

        > If changing the applications is required to work around this without a patch, then can you recommend a way to do this using the container to manage persistence?
        Short answer, create a new EntityManager for each request.

        Thanks,
        Rick

        Show
        Rick Curtis added a comment - > If changing the applications is required to work around this without a patch, then can you recommend a way to do this using the container to manage persistence? Short answer, create a new EntityManager for each request. Thanks, Rick
        Hide
        Scott Parkerson added a comment - - edited

        I, too, have run into this problem when trying to use a container-managed JPA persistence context in Apache Servicemix (Fusesource Fuse ESB). The problem occurs when using Apache Aries Blueprint JPA / JTA which creates the EntityManagerFactory for the persistence unit at bundle start, and then injects an EntityManager into each DAO that I create that is part of the persistence unit. Those DAOs are exposed as OSGi services to business services, which are then called by webservices, Camel routes, etc.

        Everything is fine until two or more threads hit an EntityManager; at that point, we get the warning to try openjpa.Multithreaded=true. Adding that to my Persistence Context configuration causes the aforementioned deadlock (same trace and everything).

        If changing the applications is required to work around this without a patch, then can you recommend a way to do this using the container to manage persistence?

        (FWIW, this is still happening as of 2.2.0; might want to update the affects version.)

        Show
        Scott Parkerson added a comment - - edited I, too, have run into this problem when trying to use a container-managed JPA persistence context in Apache Servicemix (Fusesource Fuse ESB). The problem occurs when using Apache Aries Blueprint JPA / JTA which creates the EntityManagerFactory for the persistence unit at bundle start, and then injects an EntityManager into each DAO that I create that is part of the persistence unit. Those DAOs are exposed as OSGi services to business services, which are then called by webservices, Camel routes, etc. Everything is fine until two or more threads hit an EntityManager; at that point, we get the warning to try openjpa.Multithreaded=true. Adding that to my Persistence Context configuration causes the aforementioned deadlock (same trace and everything). If changing the applications is required to work around this without a patch, then can you recommend a way to do this using the container to manage persistence? (FWIW, this is still happening as of 2.2.0; might want to update the affects version.)
        Hide
        Rick Curtis added a comment -

        Sascha -

        It's been quite a long time since I've looked at this issue... but rather than applying this patch, I'd recommend an application change. You'll be a much happier camper if you make sure that you do not share EntityManagers / managed Entities across threads. I know this isn't a very good answer, but unfortunately that is the best I can come up with ATM.

        Thanks, Rick

        Show
        Rick Curtis added a comment - Sascha - It's been quite a long time since I've looked at this issue... but rather than applying this patch, I'd recommend an application change. You'll be a much happier camper if you make sure that you do not share EntityManagers / managed Entities across threads. I know this isn't a very good answer, but unfortunately that is the best I can come up with ATM. Thanks, Rick
        Hide
        Sascha Just added a comment -

        Any updates on this issue? Running into exactly the same problem. Applying patch-1716 fixes the problem for us.
        (Running openjpa-2.1.1)

        Show
        Sascha Just added a comment - Any updates on this issue? Running into exactly the same problem. Applying patch-1716 fixes the problem for us. (Running openjpa-2.1.1)
        Hide
        Rick Curtis added a comment -

        I'm somewhat confused by OPENJPA-453 because it seems like there are two issues in that one (Correct me if I'm wrong)?

        Either way, I think this issue is related to the threading concerns that are detailed near the middle/bottom of OPENJPA-453. After reading through OPENJPA-453 I'm pretty sure that the posted patch to this JIRA isn't the right change.... There is a larger issue than this single deadlock being reported.

        I'd feel more comfortable fixing these deadlocks on a case by case basis rather than the blind "lock the Broker before the StateManager in all cases". Unfortunately at this point in time I don't have the cycles to dedicate to fixing the locking design(the root issue) of OpenJPA when running multithreaded.

        Show
        Rick Curtis added a comment - I'm somewhat confused by OPENJPA-453 because it seems like there are two issues in that one (Correct me if I'm wrong)? Either way, I think this issue is related to the threading concerns that are detailed near the middle/bottom of OPENJPA-453 . After reading through OPENJPA-453 I'm pretty sure that the posted patch to this JIRA isn't the right change.... There is a larger issue than this single deadlock being reported. I'd feel more comfortable fixing these deadlocks on a case by case basis rather than the blind "lock the Broker before the StateManager in all cases". Unfortunately at this point in time I don't have the cycles to dedicate to fixing the locking design(the root issue) of OpenJPA when running multithreaded.
        Hide
        Christiaan added a comment -

        Hi Rick, does this deadlock relate to the locking found in OPENJPA-453?

        Show
        Christiaan added a comment - Hi Rick, does this deadlock relate to the locking found in OPENJPA-453 ?
        Hide
        Rick Curtis added a comment -

        I'll have to say I'm a bit apprehensive about blinding locking the BrokerImpl every time the StateManagerImpl needs to be locked... I posted this patch so I don't lose the code while I think about it. Ideally we'd be intelligent about locking and only lock the BrokerImpl when we need to...

        Show
        Rick Curtis added a comment - I'll have to say I'm a bit apprehensive about blinding locking the BrokerImpl every time the StateManagerImpl needs to be locked... I posted this patch so I don't lose the code while I think about it. Ideally we'd be intelligent about locking and only lock the BrokerImpl when we need to...
        Hide
        Stefan Wokusch added a comment -

        thanks, nice work

        Show
        Stefan Wokusch added a comment - thanks, nice work
        Rick Curtis made changes -
        Attachment OPENJPA-1716.PATCH [ 12449593 ]
        Hide
        Rick Curtis added a comment -

        Attaching test case and fix.

        Show
        Rick Curtis added a comment - Attaching test case and fix.
        Hide
        Rick Curtis added a comment -

        Never mind, I put one together this morning.

        Show
        Rick Curtis added a comment - Never mind, I put one together this morning.
        Rick Curtis made changes -
        Field Original Value New Value
        Assignee Rick Curtis [ curtisr7 ]
        Hide
        Rick Curtis added a comment -

        Could I have you put together a simple unit test that recreates the failure?

        Show
        Rick Curtis added a comment - Could I have you put together a simple unit test that recreates the failure?
        Hide
        Stefan Wokusch added a comment -

        Ok, here are two Threads locking each other ::

        Thread [main] (Suspended)
        Unsafe.park(boolean, long) line: not available [native method]
        LockSupport.park(Object) line: 158
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 747
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquireQueued(AbstractQueuedSynchronizer$Node, int) line: 778
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquire(int) line: 1114
        ReentrantLock$NonfairSync.lock() line: 186
        ReentrantLock.lock() line: 262 [local variables unavailable]
        StateManagerImpl.lock() line: 3305
        StateManagerImpl.fetchObjectField(int) line: 2364
        StateManagerImpl.fetchObject(int) line: 2357
        RelationCollectionTableFieldStrategy(RelationToManyTableFieldStrategy).update(OpenJPAStateManager, JDBCStore, RowManager) line: 194
        RelationCollectionTableFieldStrategy.update(OpenJPAStateManager, JDBCStore, RowManager) line: 107
        FieldMapping.update(OpenJPAStateManager, JDBCStore, RowManager) line: 699
        BatchingConstraintUpdateManager(AbstractUpdateManager).update(OpenJPAStateManager, BitSet, ClassMapping, RowManager, JDBCStore, Collection, boolean) line: 334
        BatchingConstraintUpdateManager(AbstractUpdateManager).populateRowManager(OpenJPAStateManager, RowManager, JDBCStore, Collection, Collection) line: 169
        BatchingConstraintUpdateManager(AbstractUpdateManager).flush(Collection, JDBCStore, PreparedStatementManager) line: 95
        BatchingConstraintUpdateManager(AbstractUpdateManager).flush(Collection, JDBCStore) line: 76
        JDBCStoreManager.flush(Collection) line: 751
        ROPStoreManager(DelegatingStoreManager).flush(Collection<OpenJPAStateManager>) line: 131
        FinalizingBrokerImpl(BrokerImpl).flush(int) line: 2139
        FinalizingBrokerImpl(BrokerImpl).flushSafe(int) line: 2037
        FinalizingBrokerImpl(BrokerImpl).beforeCompletion() line: 1955
        LocalManagedRuntime.commit() line: 81
        FinalizingBrokerImpl(BrokerImpl).commit() line: 1479
        DelegatingBroker.commit() line: 925
        EntityManagerImpl.commit() line: 559
        ....

        Thread [pool-2-thread-1] (Suspended)
        Unsafe.park(boolean, long) line: not available [native method]
        LockSupport.park(Object) line: 158
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 747
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquireQueued(AbstractQueuedSynchronizer$Node, int) line: 778
        ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquire(int) line: 1114
        ReentrantLock$NonfairSync.lock() line: 186
        ReentrantLock.lock() line: 262
        FinalizingBrokerImpl(BrokerImpl).lock() line: 4366
        FinalizingBrokerImpl(BrokerImpl).beginOperation(boolean) line: 1893
        FinalizingBrokerImpl(BrokerImpl).isActive() line: 1865
        StateManagerImpl.beforeAccessField(int) line: 1603
        StateManagerImpl.accessingField(int) line: 1591
        ObjektEntity.pcGetid(ObjektEntity) line: not available
        TestEntity(ObjektEntity).getId() line: 91
        .....

        Show
        Stefan Wokusch added a comment - Ok, here are two Threads locking each other :: Thread [main] (Suspended) Unsafe.park(boolean, long) line: not available [native method] LockSupport.park(Object) line: 158 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 747 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquireQueued(AbstractQueuedSynchronizer$Node, int) line: 778 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquire(int) line: 1114 ReentrantLock$NonfairSync.lock() line: 186 ReentrantLock.lock() line: 262 [local variables unavailable] StateManagerImpl.lock() line: 3305 StateManagerImpl.fetchObjectField(int) line: 2364 StateManagerImpl.fetchObject(int) line: 2357 RelationCollectionTableFieldStrategy(RelationToManyTableFieldStrategy).update(OpenJPAStateManager, JDBCStore, RowManager) line: 194 RelationCollectionTableFieldStrategy.update(OpenJPAStateManager, JDBCStore, RowManager) line: 107 FieldMapping.update(OpenJPAStateManager, JDBCStore, RowManager) line: 699 BatchingConstraintUpdateManager(AbstractUpdateManager).update(OpenJPAStateManager, BitSet, ClassMapping, RowManager, JDBCStore, Collection, boolean) line: 334 BatchingConstraintUpdateManager(AbstractUpdateManager).populateRowManager(OpenJPAStateManager, RowManager, JDBCStore, Collection, Collection) line: 169 BatchingConstraintUpdateManager(AbstractUpdateManager).flush(Collection, JDBCStore, PreparedStatementManager) line: 95 BatchingConstraintUpdateManager(AbstractUpdateManager).flush(Collection, JDBCStore) line: 76 JDBCStoreManager.flush(Collection) line: 751 ROPStoreManager(DelegatingStoreManager).flush(Collection<OpenJPAStateManager>) line: 131 FinalizingBrokerImpl(BrokerImpl).flush(int) line: 2139 FinalizingBrokerImpl(BrokerImpl).flushSafe(int) line: 2037 FinalizingBrokerImpl(BrokerImpl).beforeCompletion() line: 1955 LocalManagedRuntime.commit() line: 81 FinalizingBrokerImpl(BrokerImpl).commit() line: 1479 DelegatingBroker.commit() line: 925 EntityManagerImpl.commit() line: 559 .... Thread [pool-2-thread-1] (Suspended) Unsafe.park(boolean, long) line: not available [native method] LockSupport.park(Object) line: 158 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 747 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquireQueued(AbstractQueuedSynchronizer$Node, int) line: 778 ReentrantLock$NonfairSync(AbstractQueuedSynchronizer).acquire(int) line: 1114 ReentrantLock$NonfairSync.lock() line: 186 ReentrantLock.lock() line: 262 FinalizingBrokerImpl(BrokerImpl).lock() line: 4366 FinalizingBrokerImpl(BrokerImpl).beginOperation(boolean) line: 1893 FinalizingBrokerImpl(BrokerImpl).isActive() line: 1865 StateManagerImpl.beforeAccessField(int) line: 1603 StateManagerImpl.accessingField(int) line: 1591 ObjektEntity.pcGetid(ObjektEntity) line: not available TestEntity(ObjektEntity).getId() line: 91 .....
        Hide
        Rick Curtis added a comment -

        Can you post some more information about this dead lock? At minimum the stacks for both threads involved would be a good start.

        Show
        Rick Curtis added a comment - Can you post some more information about this dead lock? At minimum the stacks for both threads involved would be a good start.
        Stefan Wokusch created issue -

          People

          • Assignee:
            Rick Curtis
            Reporter:
            Stefan Wokusch
          • Votes:
            2 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:

              Development