From: <ma...@rh...> - 2009-01-30 00:37:27
|
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head><style type="text/css"><!-- #msg DL { border : 1px #006 solid; background-color : #369; padding : 6px; color : #fff; } #msg DT { float : left; width : 6em; font-weight : bold; } #msg DL, #msg DT, #msg UL, #msg LI { font-family : arial,helvetica,sans-serif; font-size : 10pt; } h3 { font-family : arial,helvetica,sans-serif; font-size : 10pt; font-weight : bold; } #msg PRE { overflow : auto; white-space : normal; background-color : #ffc; border : 1px #fc0 solid; padding : 6px; } #msg UL, PRE, .diff { overflow : auto; } #patch h4 { font-family : arial,helvetica,sans-serif; font-size : 10pt; } #patch h4 { padding: 8px; background : #369; color : #fff; margin : 0; } #patch .propset h4, #patch .binary h4 {margin: 0;} #patch pre {padding:0;line-height:1.2em;margin:0;} #patch .diff {background:#eeeeee;padding: 0 0 10px 0;} #patch .propset .diff, #patch .binary .diff {padding: 10px 0;} #patch span {display:block;padding:0 10px;} #patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch .add {background:#ddffdd;} #patch .rem {background:#ffdddd;} #patch .lines, .info {color:#888888;background:#ffffff;} .diff { width : 100%; } #msg DL { border : 1px #006 solid; background-color : #369; padding : 6px; color : #fff; } #msg DT { float : left; width : 6em; font-weight : bold; } #msg DL, #msg DT, #msg UL, #msg LI { font-family : arial,helvetica,sans-serif; font-size : 10pt; } h3 { font-family : arial,helvetica,sans-serif; font-size : 10pt; font-weight : bold; } #msg PRE { overflow : auto; white-space : normal; background-color : #ffc; border : 1px #fc0 solid; padding : 6px; } #msg UL, PRE, .diff { overflow : auto; } #patch h4 { font-family : arial,helvetica,sans-serif; font-size : 10pt; } #patch h4 { padding: 8px; background : #369; color : #fff; margin : 0; } #patch .propset h4, #patch .binary h4 {margin: 0;} #patch pre {padding:0;line-height:1.2em;margin:0;} #patch .diff {background:#eeeeee;padding: 0 0 10px 0;} #patch .propset .diff, #patch .binary .diff {padding: 10px 0;} #patch span {display:block;padding:0 10px;} #patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch .add {background:#ddffdd;} #patch .rem {background:#ffdddd;} #patch .lines, .info {color:#888888;background:#ffffff;} .diff { width : 100%; } --></style> <title>[rhq-project.org rhq] [2836] [RHQ-1398] add a RW lock around the retrieval of the managers.</title> </head> <body> <div id="msg"> <dl> <dt>Revision</dt> <dd>2836</dd> <dt>Author</dt> <dd>mazz</dd> <dt>Date</dt> <dd>2009-01-29 18:37:25 -0600 (Thu, 29 Jan 2009)</dd> </dl> <h3>Log Message</h3> <pre>[RHQ-1398] add a RW lock around the retrieval of the managers. this helps avoid the race condition where a newly started component (started via an inventory manager thread) wants to register an event poller but the event manager hadn't been initialized yet. Now, you can't get any manager references until the PC is fully initialized</pre> <h3>Modified Paths</h3> <ul> <li><a href="#rhqtrunkmodulescoreplugincontainersrcmainjavaorgrhqcorepcPluginContainerjava">rhq/trunk/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java</a></li> </ul> </div> <div id="patch"> <h3>Diff</h3> <a id="rhqtrunkmodulescoreplugincontainersrcmainjavaorgrhqcorepcPluginContainerjava"></a> <div class="modfile"><h4>Modified: rhq/trunk/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java (2835 => 2836)</h4> <pre class="diff"> <span class="info">--- rhq/trunk/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java 2009-01-29 23:51:24 UTC (rev 2835) +++ rhq/trunk/modules/core/plugin-container/src/main/java/org/rhq/core/pc/PluginContainer.java 2009-01-30 00:37:25 UTC (rev 2836) </span><span class="lines">@@ -25,6 +25,10 @@ </span><span class="cx"> import java.io.IOException; import java.util.Collection; import java.util.LinkedHashSet; </span><span class="add">+import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; </span><span class="cx"> import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; </span><span class="lines">@@ -86,6 +90,9 @@ </span><span class="cx"> private AgentServiceStreamRemoter agentServiceStreamRemoter = null; private AgentRegistrar agentRegistrar = null; </span><span class="add">+ // this is to prevent race conditions on startup between components from all the different managers + private ReadWriteLock rwLock = new ReentrantReadWriteLock(); + </span><span class="cx"> /** * Returns the singleton instance. * </span><span class="lines">@@ -177,8 +184,11 @@ </span><span class="cx"> * @return <code>true</code> if the plugin container was initialized and started; <code>false</code> otherwise */ public boolean isStarted() { </span><span class="rem">- synchronized (INSTANCE) { </span><span class="add">+ Lock lock = obtainReadLock(); + try { </span><span class="cx"> return started; </span><span class="add">+ } finally { + releaseLock(lock); </span><span class="cx"> } } </span><span class="lines">@@ -192,7 +202,8 @@ </span><span class="cx"> * <p>If the plugin container has already been initialized, this method does nothing and returns.</p> */ public void initialize() { </span><span class="rem">- synchronized (INSTANCE) { </span><span class="add">+ Lock lock = obtainWriteLock(); + try { </span><span class="cx"> if (!started) { version = PluginContainer.class.getPackage().getImplementationVersion(); log.info("Initializing Plugin Container" + ((version != null) ? (" v" + version) : "") + "..."); </span><span class="lines">@@ -227,6 +238,8 @@ </span><span class="cx"> started = true; } </span><span class="add">+ } finally { + releaseLock(lock); </span><span class="cx"> } return; </span><span class="lines">@@ -237,7 +250,8 @@ </span><span class="cx"> * this method does nothing and returns. */ public void shutdown() { </span><span class="rem">- synchronized (INSTANCE) { </span><span class="add">+ Lock lock = obtainWriteLock(); + try { </span><span class="cx"> if (started) { eventManager.shutdown(); contentManager.shutdown(); </span><span class="lines">@@ -258,6 +272,8 @@ </span><span class="cx"> started = false; } </span><span class="add">+ } finally { + releaseLock(lock); </span><span class="cx"> } return; </span><span class="lines">@@ -295,43 +311,132 @@ </span><span class="cx"> } } </span><span class="add">+ private Lock obtainReadLock() { + // try to obtain the lock, but if we can't get the lock in 60 seconds, + // keep going. The PC is usually fine within seconds after its initializes, + // so not getting this lock within 60 seconds probably isn't detrimental. + // But if there is a deadlock, blocking forever here would be detrimental, + // so we do not do it. We'll just log a warning and let the thread keep going. + Lock readLock = rwLock.readLock(); + try { + if (!readLock.tryLock(60, TimeUnit.SECONDS)) { + String msg = "There may be a deadlock in the plugin container."; + log.warn(msg, new Throwable(msg)); + readLock = null; + } + } catch (InterruptedException e) { + readLock = null; + } + return readLock; + } + + private Lock obtainWriteLock() { + // try to obtain the lock, but if we can't get the lock in 60 seconds, + // keep going. The PC is usually fine within seconds after its initializes, + // so not getting this lock within 60 seconds probably isn't detrimental. + // But if there is a deadlock, blocking forever here would be detrimental, + // so we do not do it. We'll just log a warning and let the thread keep going. + Lock writeLock = rwLock.writeLock(); + try { + if (!writeLock.tryLock(60, TimeUnit.SECONDS)) { + String msg = "There may be a deadlock in the plugin container."; + log.warn(msg, new Throwable(msg)); + writeLock = null; + } + } catch (InterruptedException e) { + writeLock = null; + } + return writeLock; + } + + private void releaseLock(Lock lock) { + if (lock != null) { + lock.unlock(); + } + } + </span><span class="cx"> // The methods below return the actual manager implementation objects. // Only those objects inside the plugin container should be calling these getXXXManager() methods. public PluginManager getPluginManager() { </span><span class="rem">- return pluginManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return pluginManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public PluginComponentFactory getPluginComponentFactory() { </span><span class="rem">- return pluginComponentFactory; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return pluginComponentFactory; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public InventoryManager getInventoryManager() { </span><span class="rem">- return inventoryManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return inventoryManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public ConfigurationManager getConfigurationManager() { </span><span class="rem">- return configurationManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return configurationManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public MeasurementManager getMeasurementManager() { </span><span class="rem">- return measurementManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return measurementManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public OperationManager getOperationManager() { </span><span class="rem">- return operationManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return operationManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public ResourceFactoryManager getResourceFactoryManager() { </span><span class="rem">- return resourceFactoryManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return resourceFactoryManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public ContentManager getContentManager() { </span><span class="rem">- return contentManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return contentManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } public EventManager getEventManager() { </span><span class="rem">- return eventManager; </span><span class="add">+ Lock lock = obtainReadLock(); + try { + return eventManager; + } finally { + releaseLock(lock); + } </span><span class="cx"> } // The methods below return the manager implementations wrapped in their remote client interfaces. </span> </pre> </div> </div> </body> </html> |