commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dirk Verbeeck <dirk.verbe...@pandora.be>
Subject Re: cvs commit: jakarta-commons/dbcp/src/java/org/apache/commons/dbcp AbandonedObjectPool.java AbandonedTrace.java
Date Thu, 25 Sep 2003 22:57:17 GMT
I was thinking about concurrent modification of the trace List

thread1: borrowObject -> removeAbandoned (synchronized on pool) -> 
trace.iterator()
thread2: returnObject -> (synchronize on trace) -> trace.remove

This triggers a ConcurrentModificationException, IMHO

I would synchronize on the pool itself like this:
            synchronized(this) {
                trace.remove(obj);
            }

Dirk

Glenn Nielsen wrote:

> Dirk Verbeeck wrote:
>
>> Shouldn't you synchronize on the pool itself instead of on the trace 
>> list in AbandonedObjectPool?
>> borrowObject and returnObject are synchronized on trace
>> invalidateObject and removeAbandoned are synchronized on the pool
>>
>
> The synchronization required for using the GenericObjectPool is done when
> the respective super methods are called on the object pool. 
> These syncrhonizations are only for managing abandoned connections, not
> for the underlying pool.  The synchronizations were moved inside the
> methods to remove the need for a synchronization if removal of abandoned
> pools is not being used.
>
> You are right, the synchronization for invalidateObject could
> be moved inside the method like it was done for returnObject.
>
> The entire method for removeAbandoned needs to be synchronized.
>
> Regards,
>
> Glenn





Mime
View raw message