> -----Original Message----- > From: Christopher Schultz [mailto:ch...@christopherschultz.net] > Sent: Thursday, May 31, 2012 2:27 PM > To: Tomcat Users List > Subject: Re: Question about resetting datasources and changes to the > BasicDataSource.close() method > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Brooke, > > On 5/31/12 11:56 AM, Hedrick, Brooke - 43 wrote: > > /** * Not used currently */ protected boolean restarting = false; // > > bth new > > > > public void restart() { try { restarting = true; // bth new try { > > close(); } finally { restarting = false; // bth new } } catch > > (SQLException e) { log("Could not restart DataSource, cause: " + > > e.getMessage()); } } > > NB: That code isn't threadsafe. >
Chris, Are you just concerned about the restart() being called by more than 1 thread since the restarting member variable isn't protected? 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. 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. 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. 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; } --Brooke