modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java | 26 ++++++---- modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainerConfiguration.java | 2 2 files changed, 19 insertions(+), 9 deletions(-)
New commits: commit 0f973b4b8ef0f9f261a6bcc7563d823907fa5f44 Author: Jay Shaughnessy jshaughn@redhat.com Date: Wed Feb 27 12:10:54 2013 -0500
[Bug 808231 - [plugin-container] NullPointerExceptions (NPE's) occur during PC shutdown, because the PC does not wait for ExecutorServices that have been shut down to terminate] A follow-on to commit 5ef9c47ee8edab0fd0558a208911cc31dfb5d1dc. It seems this existing BZ had the same intention and actually had support built-in to deal with this problem, but had not applied it to production. So: - Enable WAIT_FOR_SHUTDOWN_SERVICE_TERMINATION_DEFAULT such that we wait 5 minutes for executors to finish their work. This is good for operation execution, avail checking, and discovery. - Keep isRunning(), added in the previous commit, but make it a lightweight check by taking away the lock request. Continue to use it to safeguard against ERROR generation even if discovery takes longer than 5 minutes to complete. - Complement the outer guards in intialize() and shutdown() with guards that are inside the lock aquisition, to prevent sequentially executing the init or shutdown logic.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java index 7c0a21b..3705505 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java @@ -260,17 +260,15 @@ public class PluginContainer {
/** * If the plugin container has been initialized, the plugins have started work, and the container is - * not actively shutting down, this returns <code>true</code>. + * not actively shutting down, this returns <code>true</code>. + * + * <p>Note! that there is no locking on this method. Therefore it returns quickly and is likely correct, but if + * called outside of locking the return value is not guaranteed. As such, use this only as a lightweight check.</p> * * @return <code>true</code> if the plugin container was initialized, started and is not shutting down; <code>false</code> otherwise */ public boolean isRunning() { - Lock lock = obtainReadLock(); - try { - return started && !shuttingDown; - } finally { - releaseLock(lock); - } + return !shuttingDown && started; }
/** @@ -283,12 +281,18 @@ public class PluginContainer { * <p>If the plugin container has already been initialized, this method does nothing and returns.</p> */ public void initialize() { + // this quick guard is OK but doesn't prevent several calls to initialize() from stacking up while waiting for the lock if (started) { log.info("Plugin container is already initialized."); }
Lock lock = obtainWriteLock(); try { + // this guard prevents us from executing initialize logic multiple times in a row + if (started) { + return; + } + version = PluginContainer.class.getPackage().getImplementationVersion(); log.info("Initializing Plugin Container" + ((version != null) ? (" v" + version) : "") + "...");
@@ -354,7 +358,8 @@ public class PluginContainer { * this method does nothing and returns. */ public boolean shutdown() { - if (!started) { + // this quick guard is OK but doesn't prevent several calls to shutdown() from stacking up while waiting for the lock + if (!isRunning()) { log.info("Plugin container is already shut down."); }
@@ -362,6 +367,11 @@ public class PluginContainer { // end up deadlocked. Lock lock = (configuration.isWaitForShutdownServiceTermination()) ? obtainReadLock() : obtainWriteLock(); try { + // this guard prevents us from executing shutdown logic multiple times in a row + if (!isRunning()) { + return true; + } + shuttingDown = true; shutdownGracefully = true; shutdownStartTime = System.currentTimeMillis(); diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainerConfiguration.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainerConfiguration.java index 41dbda7..22f8150 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainerConfiguration.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainerConfiguration.java @@ -52,7 +52,7 @@ public class PluginContainerConfiguration { private static final String CREATE_RESOURCE_CLASSLOADERS = PROP_PREFIX + "create-resource-classloaders"; private static final String START_MANAGEMENT_BEAN_PROP = PROP_PREFIX + "start-management-bean"; private static final String WAIT_FOR_SHUTDOWN_SERVICE_TERMINATION = PROP_PREFIX + "wait-for-shutdown-service-termination"; - public static final boolean WAIT_FOR_SHUTDOWN_SERVICE_TERMINATION_DEFAULT = false; + public static final boolean WAIT_FOR_SHUTDOWN_SERVICE_TERMINATION_DEFAULT = true; private static final String SHUTDOWN_SERVICE_TERMINATION_TIMEOUT = PROP_PREFIX + "shutdown-service-termination-timeout"; public static final long SHUTDOWN_SERVICE_TERMINATION_TIMEOUT_DEFAULT = 5 * 60L; // in seconds
rhq-commits@lists.fedorahosted.org