[#QB-1562] Possible deadlock in DbStore.close()

QuickBuild

Possible deadlock in DbStore.close()

Created: 26/Feb/13 02:29 PM Updated: 27/Feb/13 03:44 PM

Component/s:

None

Affects Version/s:

5.0.7

Fix Version/s:

5.0.9

Original Estimate:	Unknown	Remaining Estimate:	Unknown	Time Spent:	Unknown
Environment:	all

Description

« Hide

connection.close() can throw an SQLException - if this happens, lock will not unlock:

public void close() {
if (logger.isDebugEnabled()) {
logger.debug("Closing database " + this + " and releasing lock");
}

try {
if (connection != null) {
connection.close(); // THROW HERE
connection = null;
}

if (lock != null) {
lock.unlock(); // ...unlock skipped
lock = null;
}

ConnectionCache.INSTANCE.remove(dbDir.getAbsolutePath(), mode, method);
} catch (SQLException e) {
throw Throwables.propagate(e);
}
}

All

Comments

Work Log

Change History

Sort Order:

[ Permlink | « Hide ]

Steve Luo [27/Feb/13 02:15 PM]

Thank you very much for pointing this out.

The usage for DbStore should like below:
try {
    db.open();

    // do some db related query/update work
} finally {
    db.close();
}

So, if there is any exception occurred in open(), the close() will finally close the connection and release the lock. But it may be more safe to call close() when exception occurred during open() call.

[ Permlink | « Hide ]

Marek Baczynski [27/Feb/13 08:47 AM]

related issue is in DbStore.open(), near getConnection():

public void open(Mode mode, String method) {
if (logger.isDebugEnabled()) {
logger.debug("Trying opening the database " + this + " ...");
}

Preconditions.checkState(connection == null);
Preconditions.checkState(lock == null);

this.method = method;

if (!exists()) {
if (logger.isDebugEnabled()) {
logger.debug("Database doesn't exist, creating database "
+ this + " and opening with READ_WRITE mode");
}

this.mode = Mode.READ_WRITE;
this.lock = getLock();
this.connection = getConnection(getUrl(false)); // THROW HERE - lock does not unlock()
createDb();
...