-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brooke,

On 5/31/12 3:53 PM, Hedrick, Brooke - 43 wrote:
> Are you just concerned about the restart() being called by more
> than 1 thread since the restarting member variable isn't
> protected?

Yes.

> If so, I was looking for a short term, simple, fix.   Our tools
> that we use to reset/resize pools manage not allowing more than 1
> JMX call being sent for a single type of operation at a time.

Fair enough. Obviously, any patch to commons-dbcp would have to be
threadsafe, though.

> To really fix this, I would rather not put synchronized on the 
> restart() method because I don't want to have to wrap my brain
> around synchronized methods calling synchronized methods - hello
> potential deadlock.

Your worries are misplaced: in Java, a synchronized (non-static)
method uses the 'this' as the synchronization monitor and gets
exclusive access to that monitor before continuing (which you knew
already). When calling another synchronized method, the thread already
holds the monitor and so it gets right in -- it's essentially an
un-contended lock at that point so not only is deadlock not a concern,
the operation is very fast.

> Plus, the semantics of close already seem odd anyway.  What
> happens if one thread is running close() and another is asking for
> a connection via createDataSource().  The closed variable could be 
> false, createDataSource() would start to run, get as far as
> checking to see if the datasource is null.  Next, the close()
> method nulls out the datasource and createDataSource() returns the
> null datasource. The datasource variable itself would need to be
> "protected"/locked.

Yup: the solution is to just synchronize all the methods. If restart()
is synchronized, it will only operate while other methods are not
actively checking-out or checking-in connections. The big problem with
the concept of a "restart" is that there could be connections that are
currently checked-out during the call to recycle()... those need to be
handled in some special way -- probably added to a queue that will be
processed later to destroy the connections.

> I am not a Java expert, but this class has some oddness about what
> is synchronized.  There are getters that are synchronized.
> 
> Some examples...
> 
> public synchronized boolean getTestWhileIdle() { return 
> this.testWhileIdle; }
> 
> public synchronized long getMinEvictableIdleTimeMillis() { return 
> this.minEvictableIdleTimeMillis; }
> 
> public synchronized int getNumTestsPerEvictionRun() { return 
> this.numTestsPerEvictionRun; }

On the one hand, synchronizing these methods should not be necessary
because Java boolean values are defined to be 32-bit integers which
feature atomic assignment (that is, no thread ever sees only 8-bits of
the 32-bit value being assigned and 24-bits of the old value). On the
other hand, threads are allowed to keep cached copies of certain data
under certain conditions. Using the keyword 'volatile' /should/, in
recent JVMs, make the use of 'synchronized' completely unnecessary,
while older JVMs may even ignore 'volatile' making 'synchronized'
mandatory. Use of 'synchronized' is the safest bet because it
definitely causes a 'memory barrier' to be crossed in any version of
JVM: that means that the thread is required to synchronize (perhaps a
bad choice of words) its cache with main memory which means that all
variables get the latest copies of the "real" data.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/H7zgACgkQ9CaO5/Lv0PB/PACfTYSWPc2xxzbQB0FND9HnVMSX
v2kAoKYiQs2Tvc+uQU9RqBEX4HhFtH7d
=Mtj+
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to