From 1e96b43720e9449859c5b2bdff62a92a7a0b3d15 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Fri, 15 Sep 2017 17:34:46 -0400 Subject: [PATCH] locking fixes for H2 updates --- .../data/nvdcve/ConnectionFactory.java | 22 +++++++++++++++---- .../dependencycheck/data/nvdcve/CveDB.java | 1 - .../data/update/NvdCveUpdater.java | 11 +++++----- .../owasp/dependencycheck/utils/H2DBLock.java | 8 ++++++- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java index fc5958194..6c6111b75 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java @@ -29,10 +29,13 @@ import java.sql.SQLException; import java.sql.Statement; import javax.annotation.concurrent.ThreadSafe; import org.apache.commons.io.IOUtils; +import org.owasp.dependencycheck.data.update.exception.UpdateException; +import org.owasp.dependencycheck.exception.H2DBLockException; import org.owasp.dependencycheck.utils.DBUtils; import org.owasp.dependencycheck.utils.DependencyVersion; import org.owasp.dependencycheck.utils.DependencyVersionUtil; import org.owasp.dependencycheck.utils.FileUtils; +import org.owasp.dependencycheck.utils.H2DBLock; import org.owasp.dependencycheck.utils.Settings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -106,13 +109,20 @@ public final class ConnectionFactory { * @throws DatabaseException thrown if we are unable to connect to the * database */ - public void initialize() throws DatabaseException { + public synchronized void initialize() throws DatabaseException { //this only needs to be called once. if (connectionString != null) { return; } Connection conn = null; + H2DBLock dblock = null; try { + if (isH2Connection()) { + dblock = new H2DBLock(settings); + LOGGER.debug("locking for init"); + dblock.lock(); + } + //load the driver if necessary final String driverName = settings.getString(Settings.KEYS.DB_DRIVER_NAME, ""); if (!driverName.isEmpty()) { @@ -174,7 +184,6 @@ public final class ConnectionFactory { throw new DatabaseException("Unable to connect to the database", ex); } } - if (shouldCreateSchema) { try { createTables(conn); @@ -189,6 +198,8 @@ public final class ConnectionFactory { LOGGER.debug("", dex); throw new DatabaseException("Database schema does not match this version of dependency-check", dex); } + } catch (H2DBLockException ex) { + throw new DatabaseException("Unable to obtain an exclusive lock on the H2 database to perform initializataion", ex); } finally { if (conn != null) { try { @@ -197,6 +208,9 @@ public final class ConnectionFactory { LOGGER.debug("An error occurred closing the connection", ex); } } + if (dblock != null) { + dblock.release(); + } } } @@ -206,7 +220,7 @@ public final class ConnectionFactory { * finalize method being called as during shutdown the class loader used to * load the driver may be unloaded prior to the driver being de-registered. */ - public void cleanup() { + public synchronized void cleanup() { if (driver != null) { DriverLoader.cleanup(driver); driver = null; @@ -224,7 +238,7 @@ public final class ConnectionFactory { * @throws DatabaseException thrown if there is an exception loading the * database connection */ - public Connection getConnection() throws DatabaseException { + public synchronized Connection getConnection() throws DatabaseException { initialize(); Connection conn = null; try { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java index e1fd4623b..5dcfa4fab 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java @@ -206,7 +206,6 @@ public final class CveDB implements AutoCloseable { public CveDB(Settings settings) throws DatabaseException { this.settings = settings; connectionFactory = new ConnectionFactory(settings); - connectionFactory.initialize(); open(); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java index ec9963295..6e9a6e791 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java @@ -113,11 +113,12 @@ public class NvdCveUpdater implements CachedWebDataSource { if (isUpdateConfiguredFalse()) { return; } - H2DBLock dbupdate = null; + H2DBLock dblock = null; try { if (ConnectionFactory.isH2Connection(settings)) { - dbupdate = new H2DBLock(settings); - dbupdate.lock(); + dblock = new H2DBLock(settings); + LOGGER.debug("locking for update"); + dblock.lock(); } initializeExecutorServices(); dbProperties = cveDb.getDatabaseProperties(); @@ -142,8 +143,8 @@ public class NvdCveUpdater implements CachedWebDataSource { } catch (H2DBLockException ex) { throw new UpdateException("Unable to obtain an exclusive lock on the H2 database to perform updates", ex); } finally { - if (dbupdate != null) { - dbupdate.release(); + if (dblock != null) { + dblock.release(); } shutdownExecutorServices(); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/H2DBLock.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/H2DBLock.java index 88afce929..39a063eb2 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/H2DBLock.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/H2DBLock.java @@ -82,6 +82,9 @@ public class H2DBLock { try { final File dir = settings.getDataDirectory(); lockFile = new File(dir, "dc.update.lock"); + if (!lockFile.getParentFile().isDirectory() && !lockFile.mkdir()) { + throw new H2DBLockException("Unable to create path to data directory."); + } if (lockFile.isFile() && getFileAge(lockFile) > 5 && !lockFile.delete()) { LOGGER.warn("An old db update lock file was found but the system was unable to delete " + "the file. Consider manually deleting {}", lockFile.getAbsolutePath()); @@ -92,6 +95,7 @@ public class H2DBLock { if (!lockFile.exists() && lockFile.createNewFile()) { file = new RandomAccessFile(lockFile, "rw"); lock = file.getChannel().lock(); + LOGGER.debug("Lock file created ({})", Thread.currentThread().getName()); } } catch (IOException ex) { LOGGER.trace("Expected error as another thread has likely locked the file", ex); @@ -106,7 +110,7 @@ public class H2DBLock { } if (lock == null || !lock.isValid()) { try { - LOGGER.debug("Sleeping thread {} for 5 seconds because we could not obtain the update lock.", + LOGGER.debug("Sleeping thread {} for 5 seconds because an exclusive lock on the database could not be obtained", Thread.currentThread().getName()); Thread.sleep(5000); } catch (InterruptedException ex) { @@ -121,6 +125,7 @@ public class H2DBLock { } catch (IOException ex) { throw new H2DBLockException(ex.getMessage(), ex); } + } /** @@ -148,6 +153,7 @@ public class H2DBLock { lockFile.deleteOnExit(); } lockFile = null; + LOGGER.debug("Lock released ({})", Thread.currentThread().getName()); } /**