cayenne-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aadamc...@apache.org
Subject svn commit: r563555 - in /cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne: CayenneContext.java access/DataContext.java
Date Tue, 07 Aug 2007 17:03:14 GMT
Author: aadamchik
Date: Tue Aug  7 10:03:13 2007
New Revision: 563555

URL: http://svn.apache.org/viewvc?view=rev&rev=563555
Log:
CAY-845  DataContext.localObject() object update race condition

Modified:
    cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/CayenneContext.java
    cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/DataContext.java

Modified: cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/CayenneContext.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/CayenneContext.java?view=diff&rev=563555&r1=563554&r2=563555
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/CayenneContext.java
(original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/CayenneContext.java
Tue Aug  7 10:03:13 2007
@@ -369,59 +369,64 @@
         ClassDescriptor descriptor = getEntityResolver().getClassDescriptor(
                 id.getEntityName());
 
-        Persistent cachedObject = (Persistent) getGraphManager().getNode(id);
+        synchronized (getGraphManager()) {
+            Persistent cachedObject = (Persistent) getGraphManager().getNode(id);
 
-        // merge into an existing object
-        if (cachedObject != null) {
+            // merge into an existing object
+            if (cachedObject != null) {
 
-            // TODO: Andrus, 1/24/2006 implement smart merge for modified objects...
-            if (cachedObject != prototype
-                    && cachedObject.getPersistenceState() != PersistenceState.MODIFIED
-                    && cachedObject.getPersistenceState() != PersistenceState.DELETED)
{
+                // TODO: Andrus, 1/24/2006 implement smart merge for modified objects...
+                if (cachedObject != prototype
+                        && cachedObject.getPersistenceState() != PersistenceState.MODIFIED
+                        && cachedObject.getPersistenceState() != PersistenceState.DELETED)
{
 
-                if (prototype != null
-                        && ((Persistent) prototype).getPersistenceState() != PersistenceState.HOLLOW)
{
+                    if (prototype != null
+                            && ((Persistent) prototype).getPersistenceState() !=
PersistenceState.HOLLOW) {
 
-                    descriptor.shallowMerge(prototype, cachedObject);
+                        descriptor.shallowMerge(prototype, cachedObject);
 
-                    if (cachedObject.getPersistenceState() == PersistenceState.HOLLOW) {
-                        cachedObject.setPersistenceState(PersistenceState.COMMITTED);
+                        if (cachedObject.getPersistenceState() == PersistenceState.HOLLOW)
{
+                            cachedObject.setPersistenceState(PersistenceState.COMMITTED);
+                        }
                     }
                 }
+
+                return cachedObject;
             }
+            // create and merge into a new object
+            else {
+
+                // Andrus, 1/26/2006 - note that there is a tricky case of a temporary
+                // object
+                // passed from peer DataContext... In the past we used to throw an
+                // exception
+                // or return null. Now that we can have a valid (but generally
+                // indistinguishible) case of such object passed from parent, we let it
+                // slip... Not sure what's the best way of handling it that does not
+                // involve
+                // breaking encapsulation of the DataChannel to detect where in the
+                // hierarchy
+                // this context is.
 
-            return cachedObject;
-        }
-        // create and merge into a new object
-        else {
-
-            // Andrus, 1/26/2006 - note that there is a tricky case of a temporary object
-            // passed from peer DataContext... In the past we used to throw an exception
-            // or return null. Now that we can have a valid (but generally
-            // indistinguishible) case of such object passed from parent, we let it
-            // slip... Not sure what's the best way of handling it that does not involve
-            // breaking encapsulation of the DataChannel to detect where in the hierarchy
-            // this context is.
+                Persistent localObject;
 
-            Persistent localObject;
-            synchronized (getGraphManager()) {
                 localObject = (Persistent) descriptor.createObject();
 
                 localObject.setObjectContext(this);
                 localObject.setObjectId(id);
 
                 getGraphManager().registerNode(id, localObject);
-            }
 
-            if (prototype != null) {
-                localObject.setPersistenceState(PersistenceState.COMMITTED);
-                descriptor.shallowMerge(prototype, localObject);
-            }
-            else {
-                localObject.setPersistenceState(PersistenceState.HOLLOW);
-            }
+                if (prototype != null) {
+                    localObject.setPersistenceState(PersistenceState.COMMITTED);
+                    descriptor.shallowMerge(prototype, localObject);
+                }
+                else {
+                    localObject.setPersistenceState(PersistenceState.HOLLOW);
+                }
 
-            return localObject;
+                return localObject;
+            }
         }
 
         // ****** Copied from DataContext - end *******

Modified: cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/DataContext.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/DataContext.java?view=diff&rev=563555&r1=563554&r2=563555
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/DataContext.java
(original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/DataContext.java
Tue Aug  7 10:03:13 2007
@@ -877,7 +877,7 @@
                 return true;
             }
         });
-        
+
         // invoke callbacks
         getEntityResolver().getCallbackRegistry().performCallbacks(
                 LifecycleListener.PRE_PERSIST,
@@ -1117,7 +1117,7 @@
         int syncType = cascade
                 ? DataChannel.FLUSH_CASCADE_SYNC
                 : DataChannel.FLUSH_NOCASCADE_SYNC;
-        
+
         ObjectStore objectStore = getObjectStore();
 
         // prevent multiple commits occuring simulteneously
@@ -1569,58 +1569,62 @@
         ClassDescriptor descriptor = getEntityResolver().getClassDescriptor(
                 id.getEntityName());
 
-        Persistent cachedObject = (Persistent) getGraphManager().getNode(id);
+        // have to synchronize almost the entire method to prevent multiple threads from
+        // messing up dataobjects per CAY-845. Originally only parts of "else" were
+        // synchronized, but we had to expand the lock scope to ensure consistent
+        // behavior.
+        synchronized (getGraphManager()) {
+            Persistent cachedObject = (Persistent) getGraphManager().getNode(id);
 
-        // merge into an existing object
-        if (cachedObject != null) {
+            // merge into an existing object
+            if (cachedObject != null) {
 
-            int state = cachedObject.getPersistenceState();
+                int state = cachedObject.getPersistenceState();
 
-            // TODO: Andrus, 1/24/2006 implement smart merge for modified objects...
-            if (cachedObject != prototype
-                    && state != PersistenceState.MODIFIED
-                    && state != PersistenceState.DELETED) {
+                // TODO: Andrus, 1/24/2006 implement smart merge for modified objects...
+                if (cachedObject != prototype
+                        && state != PersistenceState.MODIFIED
+                        && state != PersistenceState.DELETED) {
 
-                descriptor.injectValueHolders(cachedObject);
+                    descriptor.injectValueHolders(cachedObject);
 
-                if (prototype != null
-                        && ((Persistent) prototype).getPersistenceState() != PersistenceState.HOLLOW)
{
+                    if (prototype != null
+                            && ((Persistent) prototype).getPersistenceState() !=
PersistenceState.HOLLOW) {
 
-                    descriptor.shallowMerge(prototype, cachedObject);
+                        descriptor.shallowMerge(prototype, cachedObject);
 
-                    if (state == PersistenceState.HOLLOW) {
-                        cachedObject.setPersistenceState(PersistenceState.COMMITTED);
+                        if (state == PersistenceState.HOLLOW) {
+                            cachedObject.setPersistenceState(PersistenceState.COMMITTED);
+                        }
                     }
                 }
-            }
 
-            return cachedObject;
-        }
-        // create and merge into a new object
-        else {
+                return cachedObject;
+            }
+            // create and merge into a new object
+            else {
 
-            Persistent localObject;
+                Persistent localObject;
 
-            synchronized (getGraphManager()) {
                 localObject = (Persistent) descriptor.createObject();
 
                 localObject.setObjectContext(this);
                 localObject.setObjectId(id);
 
                 getGraphManager().registerNode(id, localObject);
-            }
 
-            if (prototype != null
-                    && ((Persistent) prototype).getPersistenceState() != PersistenceState.HOLLOW)
{
-                localObject.setPersistenceState(PersistenceState.COMMITTED);
-                descriptor.injectValueHolders(localObject);
-                descriptor.shallowMerge(prototype, localObject);
-            }
-            else {
-                localObject.setPersistenceState(PersistenceState.HOLLOW);
-            }
+                if (prototype != null
+                        && ((Persistent) prototype).getPersistenceState() != PersistenceState.HOLLOW)
{
+                    localObject.setPersistenceState(PersistenceState.COMMITTED);
+                    descriptor.injectValueHolders(localObject);
+                    descriptor.shallowMerge(prototype, localObject);
+                }
+                else {
+                    localObject.setPersistenceState(PersistenceState.HOLLOW);
+                }
 
-            return localObject;
+                return localObject;
+            }
         }
     }
 }



Mime
View raw message