You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Kevin Menard <ni...@gmail.com> on 2008/10/24 13:20:38 UTC
Re: svn commit: r707569 - in /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src: main/java/org/apache/cayenne/ObjectContextStateLog.java test/java/org/apache/cayenne/remote/CayenneContextDeletionTest.java
Just as a heads up, we can use the new Java5 concurrent classes. I
don't think we have in the codebase, yet, but there's nothing stopping
us from doing so. Note that I haven't really looked at the problem
and if a Vector is the best fit, then that's fine, too.
--
Kevin
On Fri, Oct 24, 2008 at 3:13 AM, <an...@apache.org> wrote:
> Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java
> URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java?rev=707569&r1=707568&r2=707569&view=diff
> ==============================================================================
> --- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java (original)
> +++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java Fri Oct 24 00:13:36 2008
> @@ -23,7 +23,9 @@
> import java.util.Collection;
> import java.util.Collections;
> import java.util.HashSet;
> +import java.util.List;
> import java.util.Set;
> +import java.util.Vector;
>
> import org.apache.cayenne.graph.GraphChangeHandler;
> import org.apache.cayenne.graph.GraphManager;
> @@ -52,6 +54,11 @@
> * Updates dirty objects state and clears dirty ids map.
> */
> void graphCommitted() {
> + /**
> + * Array for deleted ids, to avoid concurrent modification
> + */
> + List deletedIds = new Vector();
> +
> for (Object id : dirtyIds) {
> Object node = graphManager.getNode(id);
> if (node instanceof Persistent) {
> @@ -62,11 +69,19 @@
> persistentNode.setPersistenceState(PersistenceState.COMMITTED);
> break;
> case PersistenceState.DELETED:
> + deletedIds.add(id);
> persistentNode.setPersistenceState(PersistenceState.TRANSIENT);
> break;
> }
> }
> }
> +
> + /**
> + * Now unregister all deleted objects
> + */
> + for (Object id : deletedIds) {
> + graphManager.unregisterNode(id);
> + }
>
> clear();
> }
Re: svn commit: r707569 - in /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src: main/java/org/apache/cayenne/ObjectContextStateLog.java test/java/org/apache/cayenne/remote/CayenneContextDeletionTest.java
Posted by Andrey Razumovsky <ra...@gmail.com>.
Iterator.remove's cool, but here I need to call side functions, which
perform deletion.
As for ArrayList/Vector, here it doesn't matter because every function can
be accessed by only one thread. We can change it if you wish.
2008/10/24, Tore Halset <ha...@pvv.ntnu.no>:
>
>
> On Oct 24, 2008, at 13:47 , Andrey Razumovsky wrote:
>
> The problem is NOT that some list is accessed by multiple threads. The
>> problem is that java throws ConcurrentModificationException when you
>> delete
>> an object from the list while iteration over it. Two solutions I know from
>> my experience are:
>> 1. First do operations, and afterwards delete (done in the code).
>> 2. Iterate through array with simple loop (e.g. for (int i = 0; i <
>> list.size(); i++) - this helps in your case, because we have only one
>> modification thread, but still I don't trust this code somewhy and try to
>> avoid it. Also here we'll need to manually change the index (i-- when
>> element is deleted).
>>
>
> 3. Iterator#remove?
>
> Also note that there is just ONE thread accessing to the list, so all
>> synchronization is useless and may result in deadlock!
>>
>
> Okay, then you should probably not use Vector that is synchronized all
> over?
>
> - Tore.
>
>
Re: svn commit: r707569 - in /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src: main/java/org/apache/cayenne/ObjectContextStateLog.java test/java/org/apache/cayenne/remote/CayenneContextDeletionTest.java
Posted by Tore Halset <ha...@pvv.ntnu.no>.
On Oct 24, 2008, at 13:47 , Andrey Razumovsky wrote:
> The problem is NOT that some list is accessed by multiple threads. The
> problem is that java throws ConcurrentModificationException when you
> delete
> an object from the list while iteration over it. Two solutions I
> know from
> my experience are:
> 1. First do operations, and afterwards delete (done in the code).
> 2. Iterate through array with simple loop (e.g. for (int i = 0; i <
> list.size(); i++) - this helps in your case, because we have only one
> modification thread, but still I don't trust this code somewhy and
> try to
> avoid it. Also here we'll need to manually change the index (i-- when
> element is deleted).
3. Iterator#remove?
> Also note that there is just ONE thread accessing to the list, so all
> synchronization is useless and may result in deadlock!
Okay, then you should probably not use Vector that is synchronized all
over?
- Tore.
Re: svn commit: r707569 - in /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src: main/java/org/apache/cayenne/ObjectContextStateLog.java test/java/org/apache/cayenne/remote/CayenneContextDeletionTest.java
Posted by Andrey Razumovsky <ra...@gmail.com>.
The problem is NOT that some list is accessed by multiple threads. The
problem is that java throws ConcurrentModificationException when you delete
an object from the list while iteration over it. Two solutions I know from
my experience are:
1. First do operations, and afterwards delete (done in the code).
2. Iterate through array with simple loop (e.g. for (int i = 0; i <
list.size(); i++) - this helps in your case, because we have only one
modification thread, but still I don't trust this code somewhy and try to
avoid it. Also here we'll need to manually change the index (i-- when
element is deleted).
Also note that there is just ONE thread accessing to the list, so all
synchronization is useless and may result in deadlock!
2008/10/24, Tore Halset <ha...@pvv.ntnu.no>:
>
> Hello.
>
> I do not know if there is a List in java.util.concurrent, but it would be
> nice if we could use one from there.
>
> Another option that seem to be preferred over Vector these days is
>
> Collections.synchronizedList(new ArrayList())
>
> Regards,
> - Tore.
>
> On Oct 24, 2008, at 13:20 , Kevin Menard wrote:
>
> Just as a heads up, we can use the new Java5 concurrent classes. I
>> don't think we have in the codebase, yet, but there's nothing stopping
>> us from doing so. Note that I haven't really looked at the problem
>> and if a Vector is the best fit, then that's fine, too.
>>
>> --
>> Kevin
>>
>>
>>
>> On Fri, Oct 24, 2008 at 3:13 AM, <an...@apache.org> wrote:
>>
>> Modified:
>>> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java
>>> URL:
>>> http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java?rev=707569&r1=707568&r2=707569&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java
>>> (original)
>>> +++
>>> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java
>>> Fri Oct 24 00:13:36 2008
>>> @@ -23,7 +23,9 @@
>>> import java.util.Collection;
>>> import java.util.Collections;
>>> import java.util.HashSet;
>>> +import java.util.List;
>>> import java.util.Set;
>>> +import java.util.Vector;
>>>
>>> import org.apache.cayenne.graph.GraphChangeHandler;
>>> import org.apache.cayenne.graph.GraphManager;
>>> @@ -52,6 +54,11 @@
>>> * Updates dirty objects state and clears dirty ids map.
>>> */
>>> void graphCommitted() {
>>> + /**
>>> + * Array for deleted ids, to avoid concurrent modification
>>> + */
>>> + List deletedIds = new Vector();
>>> +
>>> for (Object id : dirtyIds) {
>>> Object node = graphManager.getNode(id);
>>> if (node instanceof Persistent) {
>>> @@ -62,11 +69,19 @@
>>>
>>> persistentNode.setPersistenceState(PersistenceState.COMMITTED);
>>> break;
>>> case PersistenceState.DELETED:
>>> + deletedIds.add(id);
>>>
>>> persistentNode.setPersistenceState(PersistenceState.TRANSIENT);
>>> break;
>>> }
>>> }
>>> }
>>> +
>>> + /**
>>> + * Now unregister all deleted objects
>>> + */
>>> + for (Object id : deletedIds) {
>>> + graphManager.unregisterNode(id);
>>> + }
>>>
>>> clear();
>>> }
>>>
>>
>>
>
Re: svn commit: r707569 - in /cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src: main/java/org/apache/cayenne/ObjectContextStateLog.java test/java/org/apache/cayenne/remote/CayenneContextDeletionTest.java
Posted by Tore Halset <ha...@pvv.ntnu.no>.
Hello.
I do not know if there is a List in java.util.concurrent, but it would
be nice if we could use one from there.
Another option that seem to be preferred over Vector these days is
Collections.synchronizedList(new ArrayList())
Regards,
- Tore.
On Oct 24, 2008, at 13:20 , Kevin Menard wrote:
> Just as a heads up, we can use the new Java5 concurrent classes. I
> don't think we have in the codebase, yet, but there's nothing stopping
> us from doing so. Note that I haven't really looked at the problem
> and if a Vector is the best fit, then that's fine, too.
>
> --
> Kevin
>
>
>
> On Fri, Oct 24, 2008 at 3:13 AM, <an...@apache.org> wrote:
>
>> Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/
>> src/main/java/org/apache/cayenne/ObjectContextStateLog.java
>> URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/ObjectContextStateLog.java?rev=707569&r1=707568&r2=707569&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/
>> main/java/org/apache/cayenne/ObjectContextStateLog.java (original)
>> +++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/
>> main/java/org/apache/cayenne/ObjectContextStateLog.java Fri Oct 24
>> 00:13:36 2008
>> @@ -23,7 +23,9 @@
>> import java.util.Collection;
>> import java.util.Collections;
>> import java.util.HashSet;
>> +import java.util.List;
>> import java.util.Set;
>> +import java.util.Vector;
>>
>> import org.apache.cayenne.graph.GraphChangeHandler;
>> import org.apache.cayenne.graph.GraphManager;
>> @@ -52,6 +54,11 @@
>> * Updates dirty objects state and clears dirty ids map.
>> */
>> void graphCommitted() {
>> + /**
>> + * Array for deleted ids, to avoid concurrent modification
>> + */
>> + List deletedIds = new Vector();
>> +
>> for (Object id : dirtyIds) {
>> Object node = graphManager.getNode(id);
>> if (node instanceof Persistent) {
>> @@ -62,11 +69,19 @@
>>
>> persistentNode.setPersistenceState(PersistenceState.COMMITTED);
>> break;
>> case PersistenceState.DELETED:
>> + deletedIds.add(id);
>>
>> persistentNode.setPersistenceState(PersistenceState.TRANSIENT);
>> break;
>> }
>> }
>> }
>> +
>> + /**
>> + * Now unregister all deleted objects
>> + */
>> + for (Object id : deletedIds) {
>> + graphManager.unregisterNode(id);
>> + }
>>
>> clear();
>> }
>