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

Reply via email to