From: <ma...@rh...> - 2009-01-22 23:21:44
|
<!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] [2712] ensure we avoid deadlocks at all costs.</title> </head> <body> <div id="msg"> <dl> <dt>Revision</dt> <dd>2712</dd> <dt>Author</dt> <dd>mazz</dd> <dt>Date</dt> <dd>2009-01-22 17:21:41 -0600 (Thu, 22 Jan 2009)</dd> </dl> <h3>Log Message</h3> <pre>ensure we avoid deadlocks at all costs. this fixes one race condition for sure, and there is additional code to avoid the potential of others or at least help notify when it happens via log messages</pre> <h3>Modified Paths</h3> <ul> <li><a href="#rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseservercoreconcurrencyLatchedServiceControllerjava">rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/concurrency/LatchedServiceController.java</a></li> <li><a href="#rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseservercorepluginProductPluginDeployerjava">rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ProductPluginDeployer.java</a></li> </ul> </div> <div id="patch"> <h3>Diff</h3> <a id="rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseservercoreconcurrencyLatchedServiceControllerjava"></a> <div class="modfile"><h4>Modified: rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/concurrency/LatchedServiceController.java (2711 => 2712)</h4> <pre class="diff"> <span class="info">--- rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/concurrency/LatchedServiceController.java 2009-01-22 20:23:17 UTC (rev 2711) +++ rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/concurrency/LatchedServiceController.java 2009-01-22 23:21:41 UTC (rev 2712) </span><span class="lines">@@ -24,6 +24,7 @@ </span><span class="cx"> import java.util.List; import java.util.Set; import java.util.concurrent.CountDownLatch; </span><span class="add">+import java.util.concurrent.TimeUnit; </span><span class="cx"> import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; </span><span class="lines">@@ -53,8 +54,11 @@ </span><span class="cx"> checkForCircularDependencies(); // start all latched services, but they'll block </span><span class="add">+ List<Thread> threads = new ArrayList<Thread>(); </span><span class="cx"> for (LatchedService service : latchedServices) { </span><span class="rem">- new Thread(service).start(); </span><span class="add">+ Thread thread = new Thread(service, "Plugin Deployment - " + service.getServiceName()); + threads.add(thread); + thread.start(); </span><span class="cx"> } // let them all go at the same time here </span><span class="lines">@@ -62,7 +66,19 @@ </span><span class="cx"> try { // and then wait for all of them to complete </span><span class="rem">- serviceCompletionLatch.await(); </span><span class="add">+ while (!serviceCompletionLatch.await(60, TimeUnit.SECONDS)) { + boolean stillRunning = false; + for (Thread thread : threads) { + if (thread.isAlive()) { + stillRunning = true; + log.warn("Thread [" + thread.getName() + "] is still running - is it hung?"); + } + } + if (!stillRunning) { + log.error("The controller is waiting for threads that are already dead, breaking deadlock now!"); + break; + } + } </span><span class="cx"> } catch (InterruptedException ie) { log.info("Controller was interrupted; can not be sure if all services have begun"); } </span><span class="lines">@@ -121,12 +137,6 @@ </span><span class="cx"> this.serviceName = serviceName; this.dependencies = new ArrayList<LatchedService>(); this.dependees = new ArrayList<LatchedService>(); </span><span class="rem">- - /* - * so that services with no deps won't throw NPE - * when awaits on the dependencyLatch in the run method - */ - this.dependencyLatch = new CountDownLatch(0); </span><span class="cx"> } public String getServiceName() { </span><span class="lines">@@ -159,6 +169,22 @@ </span><span class="cx"> hasFailed = true; } </span><span class="add">+ // this method might get called so quickly, that the + // run method never gets the chance to create the latch + // wait here for the latch to get created + int maxWaits = 60; + while (dependencyLatch == null) { + try { + Thread.sleep(1000L); + } catch (InterruptedException e) { + } + + // avoid a deadlock (this is just more paranoia) + if (maxWaits-- <= 0) { + break; + } + } + </span><span class="cx"> dependencyLatch.countDown(); } </span><span class="lines">@@ -166,12 +192,12 @@ </span><span class="cx"> running = true; try { </span><span class="add">+ dependencyLatch = new CountDownLatch(dependencies.size()); + </span><span class="cx"> if (controller == null) { throw new IllegalStateException("LatchedServices must be started via some controller"); } </span><span class="rem">- dependencyLatch = new CountDownLatch(dependencies.size()); - </span><span class="cx"> try { /* * wait until all services are ready to begin; this is </span><span class="lines">@@ -214,12 +240,14 @@ </span><span class="cx"> } finally { // and notify dependees </span><span class="rem">- for (LatchedService dependee : dependees) { - dependee.notifyComplete(this, hasFailed); </span><span class="add">+ try { + for (LatchedService dependee : dependees) { + dependee.notifyComplete(this, hasFailed); + } + } finally { + // and notify the controller as well + controller.serviceCompletionLatch.countDown(); </span><span class="cx"> } </span><span class="rem">- - // and notify the controller as well - controller.serviceCompletionLatch.countDown(); </span><span class="cx"> } } </span></pre></div> <a id="rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseservercorepluginProductPluginDeployerjava"></a> <div class="modfile"><h4>Modified: rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ProductPluginDeployer.java (2711 => 2712)</h4> <pre class="diff"> <span class="info">--- rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ProductPluginDeployer.java 2009-01-22 20:23:17 UTC (rev 2711) +++ rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ProductPluginDeployer.java 2009-01-22 23:21:41 UTC (rev 2712) </span><span class="lines">@@ -589,7 +589,8 @@ </span><span class="cx"> pluginNotify(pluginNameDisplayName, PLUGIN_REGISTERED); } catch (Exception e) { </span><span class="rem">- log.error("Failed to register RHQ plugin [" + deploymentInfo.localUrl + "]", e); </span><span class="add">+ log.error("Failed to register RHQ plugin file [" + deploymentInfo.shortName + "] at [" + + deploymentInfo.localUrl + "]", e); </span><span class="cx"> } } </span> </pre> </div> </div> </body> </html> |