From cd4f09dc86adfdbf6c19becc2af3dbe116cc568d Mon Sep 17 00:00:00 2001 From: Stefan Neuhaus Date: Sat, 11 Feb 2017 20:16:24 +0100 Subject: [PATCH] NvdCveUpdater: Refactor thread pool concept - Make thread pools members of the class to facilitate reuse - Increase default max download thread pool size from 3 to 50 (should be fine for mostly blocking tasks like downloading) --- .../data/update/NvdCveUpdater.java | 57 ++++++++++++------- .../main/resources/dependencycheck.properties | 2 +- .../test/resources/dependencycheck.properties | 2 +- .../test/resources/dependencycheck.properties | 2 +- 4 files changed, 38 insertions(+), 25 deletions(-) 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 bcf19a34b..93cdbcf1e 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 @@ -54,9 +54,21 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { */ private static final Logger LOGGER = LoggerFactory.getLogger(NvdCveUpdater.class); /** - * The max thread pool size to use when downloading files. + * The thread pool size to use for CPU-intense tasks. */ - public static final int MAX_THREAD_POOL_SIZE = Settings.getInt(Settings.KEYS.MAX_DOWNLOAD_THREAD_POOL_SIZE, 3); + private static final int PROCESSING_THREAD_POOL_SIZE = 1; + /** + * The thread pool size to use when downloading files. + */ + private static final int DOWNLOAD_THREAD_POOL_SIZE = Settings.getInt(Settings.KEYS.MAX_DOWNLOAD_THREAD_POOL_SIZE, 50); + /** + * ExecutorService for CPU-intense processing tasks. + */ + private ExecutorService processingExecutorService = null; + /** + * ExecutorService for tasks that involve blocking activities and are not very CPU-intense, e.g. downloading files. + */ + private ExecutorService downloadExecutorService = null; /** * Downloads the latest NVD CVE XML file from the web and imports it into @@ -72,10 +84,11 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { return; } } catch (InvalidSettingException ex) { - LOGGER.trace("inavlid setting UPDATE_NVDCVE_ENABLED", ex); + LOGGER.trace("invalid setting UPDATE_NVDCVE_ENABLED", ex); } try { + initializeExecutorServices(); openDataStores(); boolean autoUpdate = true; try { @@ -101,10 +114,27 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { } throw new UpdateException("Unable to download the NVD CVE data.", ex); } finally { + shutdownExecutorServices(); closeDataStores(); } } + private void initializeExecutorServices() { + processingExecutorService = Executors.newFixedThreadPool(PROCESSING_THREAD_POOL_SIZE); + downloadExecutorService = Executors.newFixedThreadPool(DOWNLOAD_THREAD_POOL_SIZE); + LOGGER.debug("#download threads: {}", DOWNLOAD_THREAD_POOL_SIZE); + LOGGER.debug("#processing threads: {}", PROCESSING_THREAD_POOL_SIZE); + } + + private void shutdownExecutorServices() { + if (processingExecutorService != null) { + processingExecutorService.shutdownNow(); + } + if (downloadExecutorService != null) { + downloadExecutorService.shutdownNow(); + } + } + /** * Checks if the NVD CVE XML files were last checked recently. As an * optimization, we can avoid repetitive checks against the NVD. Setting @@ -178,18 +208,13 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { LOGGER.info("NVD CVE requires several updates; this could take a couple of minutes."); } - final int poolSize = (MAX_THREAD_POOL_SIZE < maxUpdates) ? MAX_THREAD_POOL_SIZE : maxUpdates; - - final ExecutorService downloadExecutors = Executors.newFixedThreadPool(poolSize); - final ExecutorService processExecutor = Executors.newSingleThreadExecutor(); final Set>> downloadFutures = new HashSet>>(maxUpdates); for (NvdCveInfo cve : updateable) { if (cve.getNeedsUpdate()) { - final DownloadTask call = new DownloadTask(cve, processExecutor, getCveDB(), Settings.getInstance()); - downloadFutures.add(downloadExecutors.submit(call)); + final DownloadTask call = new DownloadTask(cve, processingExecutorService, getCveDB(), Settings.getInstance()); + downloadFutures.add(downloadExecutorService.submit(call)); } } - downloadExecutors.shutdown(); //next, move the future future processTasks to just future processTasks final Set> processFutures = new HashSet>(maxUpdates); @@ -198,21 +223,13 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { try { task = future.get(); } catch (InterruptedException ex) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); - LOGGER.debug("Thread was interrupted during download", ex); throw new UpdateException("The download was interrupted", ex); } catch (ExecutionException ex) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); - LOGGER.debug("Thread was interrupted during download execution", ex); throw new UpdateException("The execution of the download was interrupted", ex); } if (task == null) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); LOGGER.debug("Thread was interrupted during download"); throw new UpdateException("The download was interrupted; unable to complete the update"); } else { @@ -227,15 +244,11 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { throw task.getException(); } } catch (InterruptedException ex) { - processExecutor.shutdownNow(); LOGGER.debug("Thread was interrupted during processing", ex); throw new UpdateException(ex); } catch (ExecutionException ex) { - processExecutor.shutdownNow(); LOGGER.debug("Execution Exception during process", ex); throw new UpdateException(ex); - } finally { - processExecutor.shutdown(); } } diff --git a/dependency-check-core/src/main/resources/dependencycheck.properties b/dependency-check-core/src/main/resources/dependencycheck.properties index 21db405f5..d4b6b19e1 100644 --- a/dependency-check-core/src/main/resources/dependencycheck.properties +++ b/dependency-check-core/src/main/resources/dependencycheck.properties @@ -1,7 +1,7 @@ application.name=${pom.name} application.version=${pom.version} autoupdate=true -max.download.threads=3 +max.download.threads=50 # the url to obtain the current engine version from engine.version.url=https://jeremylong.github.io/DependencyCheck/current.txt diff --git a/dependency-check-core/src/test/resources/dependencycheck.properties b/dependency-check-core/src/test/resources/dependencycheck.properties index 138bb587c..277b1e6ad 100644 --- a/dependency-check-core/src/test/resources/dependencycheck.properties +++ b/dependency-check-core/src/test/resources/dependencycheck.properties @@ -1,7 +1,7 @@ application.name=${pom.name} application.version=${pom.version} autoupdate=true -max.download.threads=3 +max.download.threads=50 # the url to obtain the current engine version from engine.version.url=https://jeremylong.github.io/DependencyCheck/current.txt diff --git a/dependency-check-utils/src/test/resources/dependencycheck.properties b/dependency-check-utils/src/test/resources/dependencycheck.properties index f580b54e8..69b649e92 100644 --- a/dependency-check-utils/src/test/resources/dependencycheck.properties +++ b/dependency-check-utils/src/test/resources/dependencycheck.properties @@ -1,7 +1,7 @@ application.name=${pom.name} application.version=${pom.version} autoupdate=true -max.download.threads=3 +max.download.threads=50 # the url to obtain the current engine version from engine.version.url=http://jeremylong.github.io/DependencyCheck/current.txt