You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Ravi Palacherla <ra...@oracle.com> on 2009/07/02 20:00:16 UTC

Reason for introducing lock inside statemanagerImpl.

Hi Pinaki,

 

I have a question regarding lock inside statemanagerImpl.

I see that it is introduced as part of HYPERLINK "https://issues.apache.org/jira/browse/OPENJPA-825"OPENJPA-825 ( r727297).

Before that change statemanager used to rely on broker's lock.

As part of openjpa-825, a separate lock has been introduced for statemanager.

 

Can you please help me understand the reason for introducing a separate lock for statemanager.

 

Reason for my question :

 

I am working on OPENJPA-453 on trunk ;  the cause of the issue on trunk is 

Thread0 takes a reentrant lock inside BrokerImpl and waits to acquire reentrant lock inside statemanagerImpl. 
Thread1 takes a reentrant lock inside StatemanagerImpl and waits to acquire reentrant lock inside BrokerImpl.

This is causing a deadlock.

 

Details of the issue are under:

 https://issues.apache.org/jira/browse/OPENJPA-453#action_12725820

 

A test case demonstrating the issue is  attached to the same JIRA  (HYPERLINK "https://issues.apache.org/jira/secure/attachment/12412408/OPENJPA-453_trunk_testcase.patch"OPENJPA-453_trunk_testcase.patch).

 

There are two fixes I can think of to avoid the above issue:

 

 

1)      To obtain broker's lock before obtaining SM's lock inside StateManagerImpl.lock()

Here is an svn diff for this fix.

 

Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java

===================================================================

--- openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java       (revision 790413)

+++ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java    (working copy)

@@ -3248,16 +3248,20 @@

      * Lock the state manager if the multithreaded option is set.

      */

     protected void lock() {

-        if (_instanceLock != null)

+        if (_instanceLock != null) {

+            _broker.lock();

                        _instanceLock.lock();

+        }

     }

 

     /**

      * Unlock the state manager.

      */

        protected void unlock () {

-        if (_instanceLock != null)

-                      _instanceLock.unlock();

+        if (_instanceLock != null) {

+            _instanceLock.unlock();

+            _broker.unlock();

+        }

        }

 

     private void writeObject(ObjectOutputStream oos) throws IOException {

 

 

2)      Second is to go back to the original locking mechanism , that is statemanger will use broker's lock.

 

Code for SM's lock and unlock will be:

 

protected void lock() {

        _broker.lock();

    }

 

                                protected void unlock () {

       _broker.unlock ();

    }

   

 

Before deciding on which path to take, I thought I have to understand the reason for introducing separate lock for statemanagerImpl.

 

Thanks in advance,

Ravi.

Re: Reason for introducing lock inside statemanagerImpl.

Posted by Pinaki Poddar <pp...@us.ibm.com>.
Hi Ravi,
   try (2).

   the change was brought in (if memory serves me right) for the case 
where multiple threads access a StateManager within a *single* broker. 
This happended when SliceStoreManager spawns threads to execute database 
operations. but after i made this change, i did modify further threading 
model behavior for Slice threads -- so this instance-level lock to 
StateManager *may be* redundant. 

   if you are trying (2), the multithreaded Slice tests are in slice 
module.

 
Regards --

Pinaki 








Ravi Palacherla <ra...@oracle.com> 
07/02/2009 01:00 PM

To
Pinaki Poddar <pp...@apache.org>
cc
dev@openjpa.apache.org
Subject
Reason for introducing lock inside statemanagerImpl.






Hi Pinaki,
 
I have a question regarding lock inside statemanagerImpl.
I see that it is introduced as part of OPENJPA-825 ( r727297).
Before that change statemanager used to rely on broker’s lock.
As part of openjpa-825, a separate lock has been introduced for 
statemanager.
 
Can you please help me understand the reason for introducing a separate 
lock for statemanager.
 
Reason for my question :
 
I am working on OPENJPA-453 on trunk ;  the cause of the issue on trunk is 

Thread0 takes a reentrant lock inside BrokerImpl and waits to acquire 
reentrant lock inside statemanagerImpl. 
Thread1 takes a reentrant lock inside StatemanagerImpl and waits to 
acquire reentrant lock inside BrokerImpl.
This is causing a deadlock.
 
Details of the issue are under:
 https://issues.apache.org/jira/browse/OPENJPA-453#action_12725820
 
A test case demonstrating the issue is  attached to the same JIRA  (
OPENJPA-453_trunk_testcase.patch).
 
There are two fixes I can think of to avoid the above issue:
 
 
1)      To obtain broker’s lock before obtaining SM’s lock inside 
StateManagerImpl.lock()
Here is an svn diff for this fix.
 
Index: 
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java
===================================================================
--- 
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java 
      (revision 790413)
+++ 
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java 
   (working copy)
@@ -3248,16 +3248,20 @@
      * Lock the state manager if the multithreaded option is set.
      */
     protected void lock() {
-        if (_instanceLock != null)
+        if (_instanceLock != null) {
+            _broker.lock();
                        _instanceLock.lock();
+        }
     }
 
     /**
      * Unlock the state manager.
      */
        protected void unlock () {
-        if (_instanceLock != null)
-                      _instanceLock.unlock();
+        if (_instanceLock != null) {
+            _instanceLock.unlock();
+            _broker.unlock();
+        }
        }
 
     private void writeObject(ObjectOutputStream oos) throws IOException {
 
 
2)      Second is to go back to the original locking mechanism , that is 
statemanger will use broker’s lock.
 
Code for SM’s lock and unlock will be:
 
protected void lock() {
        _broker.lock();
    }
 
                                protected void unlock () {
       _broker.unlock ();
    }
 
 
Before deciding on which path to take, I thought I have to understand the 
reason for introducing separate lock for statemanagerImpl.
 
Thanks in advance,
Ravi.