db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Armin Waibel <arm...@apache.org>
Subject Re: Bug and proposed patch
Date Mon, 13 Dec 2004 13:03:40 GMT
Hi Tino,

Tino Schöllhorn wrote:

> Hi Jakob,
> 
> it would be great if you could integrate it. Where exactly the element 
> is removed from the collection I do not know either. I'd thought you 
> could help here. But anyway: if the modified code is checked in it works 
> great and should'nt have any side effects.
>

Think we need an test case to reproduce your test. Could you modify an 
existing test (e.g. using CollectionTests) to reproduce your problem?

As PB-api is not thread-safe (each thread have to use it's own PB 
instance) the error could only happen when OJB itself try to remove an 
object from the Iterator. If this is the case OJB has a bug in handling 
deleted objects and should be fixed IMO without copy the referenced 
objects (if this will not be possible, your patch should be applied).

regards,
Armin


> With regards
> Tino
> 
> 
> 
> Jakob Braeuchi wrote:
> 
>> hit tino, chris,
>>
>> i can integrate your patch into delteCollections(), but i still do not 
>> see
>> where the element is removed from the collection. doDelete() deletes the
>> item from the db.
>>
>> jakob
>>
>>
>>> Hi Chris,
>>>
>>> you're absolutely right and exactly that happens in the 
>>> PersistenceBrokerImpl#doDelete method. The tricky thing is though 
>>> that not every Iterator-type supports the remove method. So I 
>>> proposed a patch that works even if the used Iterator doesn't support
>>> Iterator#remove.
>>>
>>> With regards
>>> Tino
>>>
>>> Chris Lamprecht wrote:
>>>
>>>> It can happen with two threads, but it can also happen in this
>>>> situation (which I've seen several times):
>>>>
>>>> Iterator iterator = myCollection.iterator();
>>>> while (iterator.hasNext()) {
>>>>   Object foo = iterator.next();
>>>>   if (something is true about foo) {
>>>>      myCollection.remove(foo);     // INCORRECT!  Must call
>>>> iterator.remove() instead
>>>>   }
>>>> }
>>>>
>>>> So the mistake is, you can't call remove() on a collection while
>>>> you're iterating through it -- instead you need to call
>>>> iterator.remove(), which removes the last object you got from
>>>> iterator.next().  It won't complain on the call to
>>>> collection.remove(), but next time you call iterator.next(), it will
>>>> throw a ConcurrentModificationException.
>>>>
>>>> -Chris
>>>>
>>>> On Mon, 13 Dec 2004 00:20:15 +0100, Oliver Zeigermann
>>>> <oliver.zeigermann@gmail.com> wrote:
>>>>
>>>>
>>>>> Hi Tino,
>>>>>
>>>>> I am a newbie to OJB, but AFAIK a concurrent modification exception
>>>>> will be thrown only when there are two concurrent changes to the same
>>>>> list. This can only be the case when there are two threads working on
>>>>> the same collection. How can this happen?
>>>>>
>>>>> Can you explain why you think your patch works?
>>>>>
>>>>> Oliver
>>>>>
>>>>> On Sun, 12 Dec 2004 12:13:48 +0100, Tino Schöllhorn
>>>>>
>>>>>
>>>>> <t.schoellhorn@plattform-gmbh.de> wrote:
>>>>>
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> yesterday I posted a problem in the user-list (Deleting 
>>>>>> Main-Object and
>>>>>> its association-objects). Currently there are no answers for this
>>>>>> message. I looked further at the code of OJB. The method which is
>>>>>> interesting at this point is
>>>>>> PersistenceBrokerImpl.deleteCollections(Object, List).
>>>>>>
>>>>>> Suppose you have the following case: Persons work at Projects with
an
>>>>>> associated Role. Now if you are deleting a Person-Object or a
>>>>>> Project-Object you want to delete the corresponding Role-objects
as
>>>
>>>
>>> well.
>>>
>>>>>> Normally one can achieve that with *auto-delete=object* at the
>>>>>> collections for the role-objects. But the current implementation
does
>>>>>> not work here correctly. If one has 3 or more role objects to be
>>>
>>>
>>> handled
>>>
>>>>>> one gets a ConcurrentModificationException, like this one (line 
>>>>>> numbers
>>>>>> have changed because of some change from me):
>>>>>>
>>>>>> java.util.ConcurrentModificationException
>>>>>>       at
>>>
>>>
>>> java.util.AbstractList$Itr.checkForComodification(AbstractList.java:448)
>>>
>>>>>>       at java.util.AbstractList$Itr.next(AbstractList.java:419)
>>>>>>       at
>>>
>>>
>>>>> org.apache.ojb.broker.core.PersistenceBrokerImpl.deleteCollections(PersistenceBrokerImpl.java:701)

>>>>>
>>>>>
>>>>>>       at
>>>
>>>
>>>>> org.apache.ojb.broker.core.PersistenceBrokerImpl.doDelete(PersistenceBrokerImpl.java:514)

>>>>>
>>>>>
>>>>>>       at
>>>
>>>
>>>>> org.apache.ojb.broker.core.PersistenceBrokerImpl.delete(PersistenceBrokerImpl.java:466)

>>>>>
>>>>>
>>>>>>       at
>>>
>>>
>>>>> org.apache.ojb.broker.core.DelegatingPersistenceBroker.delete(DelegatingPersistenceBroker.java:170)

>>>>>
>>>>>
>>>>>>       at
>>>
>>>
>>>>> org.apache.ojb.broker.core.DelegatingPersistenceBroker.delete(DelegatingPersistenceBroker.java:170)

>>>>>
>>>>>
>>>>>>       at kos.generator.DataObject.delete(DataObject.java:106)
>>>>>>       at
>>>
>>>
>>> kos.intranet2.sandbox.DeletePersonTest.runTest(DeletePersonTest.java:64)
>>>
>>>>>>       at
>>>
>>>
>>> kos.intranet2.sandbox.DeletePersonTest.main(DeletePersonTest.java:98)
>>>
>>>>>> The current implementation looks like:
>>>>>>
>>>>>> if (cds.getCascadingDelete() ==
>>>
>>>
>>> bjectReferenceDescriptor.CASCADE_OBJECT)
>>>
>>>>>> {
>>>>>>       Object col = cds.getPersistentField().get(obj);
>>>>>>        if (col != null)
>>>>>>        {
>>>>>>               while (colIterator.hasNext())
>>>>>>                {
>>>>>>                       doDelete(colIterator.next());
>>>>>>                }
>>>>>>        }
>>>>>> }
>>>>>>
>>>>>> And the ConcurrentModificationException is thrown at the call of
>>>
>>>
>>> doDelete.
>>>
>>>>>> If a am using OJB correctly this use-case should work. So I 
>>>>>> propose the
>>>>>> following (but perhaps imperformant and inelegant) patch: The idea
is
>>>>>> simple. Instead of deleting the object immediately from its
>>>>>> origin-collection, copy the elements to another collection an use

>>>>>> this
>>>>>> for deleting.
>>>>>>
>>>>>> if (cds.getCascadingDelete() ==
>>>
>>>
>>> bjectReferenceDescriptor.CASCADE_OBJECT)
>>>
>>>>>> {
>>>>>>       Object col = cds.getPersistentField().get(obj);
>>>>>>        if (col != null)
>>>>>>        {
>>>>>>               // Inelegant and brutal. but it works
>>>>>>                Collection c = new java.util.ArrayList();
>>>>>>                 while (colIterator.hasNext()) {
>>>>>>                       c.add(colIterator.next());
>>>>>>                 }
>>>>>>                 Iterator j = c.iterator();
>>>>>>                 while (j.hasNext())
>>>>>>                 {
>>>>>>                       doDelete(j.next());
>>>>>>                 }
>>>>>>        }
>>>>>> }
>>>>>>
>>>>>> So do you think you could apply this patch - or am I mistaken and

>>>>>> I am
>>>>>> using OJB in a wrong way?
>>>>>>
>>>>>> With regards
>>>>>> Tino
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>>>>> For additional commands, e-mail: ojb-dev-help@db.apache.org
>>>>>>
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>>>> For additional commands, e-mail: ojb-dev-help@db.apache.org
>>>>>
>>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>> For additional commands, e-mail: ojb-dev-help@db.apache.org
>>>
>>
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Mime
View raw message