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