tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hedrick, Brooke - 43" <brooke.hedr...@rainhail.com>
Subject RE: Question about resetting datasources and changes to the BasicDataSource.close() method
Date Thu, 31 May 2012 19:53:11 GMT

> -----Original Message-----
> From: Christopher Schultz [mailto:chris@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

Mime
View raw message