Mailing-List: contact dev-help@openjpa.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@openjpa.apache.org
Received-SPF: pass (nike.apache.org: local policy)
MIME-Version: 1.0
Message-ID: <591b6134-c2ca-4b0a-bb2a-02cab62557df@default>
Date: Thu, 2 Jul 2009 11:00:16 -0700 (PDT)
From: Ravi Palacherla <ravi.palacherla@oracle.com>
To: Pinaki Poddar <ppoddar@apache.org>
Cc: dev@openjpa.apache.org
Subject: Reason for introducing lock inside statemanagerImpl.
Content-Type: multipart/alternative;
 boundary="__12465576185488421abhmt006.oracle.com"

--__12465576185488421abhmt006.oracle.com
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable

Hi Pinaki,

=20

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 statemanage=
r.

=20

Can you please help me understand the reason for introducing a separate loc=
k for statemanager.

=20

Reason for my question :

=20

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

Thread0 takes a reentrant lock inside BrokerImpl and waits to acquire reent=
rant lock inside statemanagerImpl.=20
Thread1 takes a reentrant lock inside StatemanagerImpl and waits to acquire=
 reentrant lock inside BrokerImpl.

This is causing a deadlock.

=20

Details of the issue are under:

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

=20

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

=20

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

=20

=20

1)      To obtain broker's lock before obtaining SM's lock inside StateMana=
gerImpl.lock()

Here is an svn diff for this fix.

=20

Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerI=
mpl.java

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

--- 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 !=3D null)

+        if (_instanceLock !=3D null) {

+            _broker.lock();

                        _instanceLock.lock();

+        }

     }

=20

     /**

      * Unlock the state manager.

      */

        protected void unlock () {

-        if (_instanceLock !=3D null)

-                      _instanceLock.unlock();

+        if (_instanceLock !=3D null) {

+            _instanceLock.unlock();

+            _broker.unlock();

+        }

        }

=20

     private void writeObject(ObjectOutputStream oos) throws IOException {

=20

=20

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

=20

Code for SM's lock and unlock will be:

=20

protected void lock() {

        _broker.lock();

    }

=20

                                protected void unlock () {

       _broker.unlock ();

    }

  =20

=20

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

=20

Thanks in advance,

Ravi.

--__12465576185488421abhmt006.oracle.com--