Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Message-ID: <3F3CD4BE.5050907@pandora.be>
Date: Fri, 15 Aug 2003 14:40:30 +0200
From: Dirk Verbeeck <dirk.verbeeck@pandora.be>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
 rv:1.0.2) Gecko/20030208 Netscape/7.02
MIME-Version: 1.0
To: Jakarta Commons Developers List <commons-dev@jakarta.apache.org>
Subject: Re: [pool] faulty change of synchronization? - Re: cvs commit:
 jakarta-commons/pool/src/java/org/apache/commons/pool/impl
 GenericObjectPool.java
References: <20030813193210.74504.qmail@web20706.mail.yahoo.com>
 <3F3AAA72.8040103@pandora.be>
 <Pine.NEB.4.53.0308141612450.23810@rhombus.znep.com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Hi Marc

Very nice link you gave, very interesting.
The changes I'm doing is a work in progress. Before releasing it we will 
do a formal review including a vote,
beta test on release canidate and then the actual release. You will get 
enough time to review.

But the code is not very good yet. There should be a simpler way to 
solve these synchronization issues.
I'll have another go at it...

Dirk


Marc Slemko wrote:

>jOn Wed, 13 Aug 2003, Dirk Verbeeck wrote:
>
>  
>
>>_numActive is an int so the synchronized block wasn't needed.
>>But the extra "synchronized" is in the case where a new object
>>has to be created and not in the critical "get from pool" code path.
>>
>>All the success code paths have now one synchronized block.
>>I'll leave it as is for now.
>>    
>>
>
>No, you need the synchronization (or some other thread safe approach),
>otherwise you risk all sorts of errors.  There are a very limited
>set of cases where you can avoid synchronization when you need concurrent
>access from multiple threads.
>
>Synchronization in java handles both mutual exclusion and visibility.
>There is no guarantee that a change made in one thread to a variable
>is visible to another without synchronization.
>
>See some of the references linked to from:
>
>	http://www.cs.umd.edu/~pugh/java/memoryModel/
>
>There appear to be all sorts of race conditions in borrowObject() now...
>_numActive, accesses to _pool, _maxActive... you can't check the value of
>a counter outside a synchronized block then only synchronize to modify it!
>
>I think you need to step back and reconsider what problems these changes
>are trying to fix.  You need to find the real operation that is
>bottlenecking things (which for connection pools would normally be calling
>into the JDBC driver to open a connection), and move that operation
>outside the synchronized block without doing unsafe things.
>
>Now, ideally that whole thing would be reworked even further to have the
>connection opener in a seperate thread to allow the getConnection() to
>time out after a specified timeout, while it keeps trying to open the
>connection in the background.  In fact, if you do that you may be able to
>just leave the whole method synchronized and wait on the opener thread,
>but there are some extra efforts required to ensure fairness, etc. that
>may make that more complex.
>
>I strongly recommend you roll back the synchronization changes unless
>someone who fully understands what has to be synchronized and why can
>review possible fixes, I don't have time just now, maybe next week; the
>current state is _NOT_ good enough.
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>
>
>  
>


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