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:51:32 GMT
Tino Schöllhorn wrote:

> Hi Armin,
> 
> I can reproduce this behavior in a scenario which is single-threaded and 
> it uses only one PB.

ok

> For the test case: I will try to provide a test 
> case where one can reproduce this behaviour but I think I will need your 
> help there: for example I do not know how I can run your existing test 
> cases and what db they are using.

if you are using OJB source version then you could run each test case in 
your IDE.
1. call "ant prepare-testdb" to setup OJB test bed.
2. Run the test, e.g. CollectionsTest.java in your IDE, set working 
directory to ....db-ojb/target/test/ojb


> 
> To my proposed fix: I also think that this fix is not very elegant, but 
> it works. And I can't find an argument why one should not integrate this 
> fix in the 1.0_BRANCH.
>

I would apply your patch as a last resort, because it only mask a more 
serious design problem (if you right and there was an bug ;-)).

regards,
Armin

> regards,
> Tino
> 
> 
> Armin Waibel wrote:
> 
>> 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
> 
> 
> 

---------------------------------------------------------------------
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