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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
