.classpath |
2
modules/core/native-system/src/main/java/org/rhq/core/system/JavaSystemInfo.java |
22 ++--
modules/core/native-system/src/main/java/org/rhq/core/system/SystemInfoFactory.java |
37 +++++++
modules/core/native-system/src/test/java/org/rhq/core/system/NativeSystemInfoTest.java |
2
modules/core/util/src/main/java/org/rhq/core/util/exec/ProcessExecutor.java |
47 +++++++---
modules/core/util/src/main/java/org/rhq/core/util/exec/StreamRedirector.java |
13 +-
6 files changed, 92 insertions(+), 31 deletions(-)
New commits:
commit 26063f9f8e50ac58d774959a42d13b6244265e7b
Author: John Mazzitelli <mazz(a)redhat.com>
Date: Thu Jun 27 13:56:06 2013 -0400
fix eclipse classpath
diff --git a/.classpath b/.classpath
index 70d5f0a..d0646ed 100644
--- a/.classpath
+++ b/.classpath
@@ -324,6 +324,8 @@
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/jboss-dmr/1.1.1.Final/jboss-dmr-1.1.1.Final.jar"/>
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/as/jboss-as-naming/7.2.0.Alpha1-redhat-4/jboss-as-naming-7.2.0.Alpha1-redhat-4.jar"/>
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/as/jboss-as-server/7.2.0.Alpha1-redhat-4/jboss-as-server-7.2.0.Alpha1-redhat-4.jar"/>
+ <classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/as/jboss-as-ee/7.2.0.Alpha1-redhat-4/jboss-as-ee-7.2.0.Alpha1-redhat-4.jar"/>
+ <classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/as/jboss-as-ejb3/7.2.0.Alpha1-redhat-4/jboss-as-ejb3-7.2.0.Alpha1-redhat-4.jar"/>
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/staxmapper/1.1.0.Final/staxmapper-1.1.0.Final.jar"/>
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/modules/jboss-modules/1.1.1.GA/jboss-modules-1.1.1.GA.jar"/>
<classpathentry exported="true" kind="var"
path="M2_REPO/org/jboss/as/jboss-as-subsystem-test/7.1.1.Final/jboss-as-subsystem-test-7.1.1.Final.jar"/>
commit b361a1b07c7d14834fcf4986e09f7bf683be4bf8
Author: Elias Ross <elias_ross(a)apple.com>
Date: Tue Nov 6 14:24:33 2012 -0800
Bug 872738 - Agent awaits shutdown of ProcessExecutor idle worker threads;
Should terminate immediately
The agent hangs if the thread pool was used
diff --git
a/modules/core/native-system/src/main/java/org/rhq/core/system/JavaSystemInfo.java
b/modules/core/native-system/src/main/java/org/rhq/core/system/JavaSystemInfo.java
index 4659945..f4e3e3c 100644
--- a/modules/core/native-system/src/main/java/org/rhq/core/system/JavaSystemInfo.java
+++ b/modules/core/native-system/src/main/java/org/rhq/core/system/JavaSystemInfo.java
@@ -30,6 +30,7 @@ import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.List;
import java.util.Locale;
+import java.util.concurrent.ExecutorService;
import org.hyperic.sigar.Mem;
import org.hyperic.sigar.NetConnection;
@@ -48,6 +49,19 @@ import org.rhq.core.util.exec.ProcessToStart;
* @author John Mazzitelli
*/
public class JavaSystemInfo implements SystemInfo {
+
+ private final ProcessExecutor javaExec;
+
+ /**
+ * Constructor for {@link JavaSystemInfo} with package scope so only the {@link
SystemInfoFactory} can instantiate
+ * this object.
+ *
+ * @param threadPool executor thread pool used for managing sub processes
+ */
+ JavaSystemInfo(ExecutorService threadPool) {
+ javaExec = new ProcessExecutor(threadPool);
+ }
+
/**
* Returns <code>false</code> - this implementation relies solely on the
Java API to get all the system information
* it can.
@@ -194,7 +208,6 @@ public class JavaSystemInfo implements SystemInfo {
executionResults.setCapturedOutputStream(outputStream);
}
- ProcessExecutor javaExec = new ProcessExecutor();
ProcessExecutorResults javaExecResults = javaExec.execute(process);
executionResults.setExitCode(javaExecResults.getExitCode());
executionResults.setError(javaExecResults.getError());
@@ -290,13 +303,6 @@ public class JavaSystemInfo implements SystemInfo {
throw getUnsupportedException("Cannot get network connections");
}
- /**
- * Constructor for {@link JavaSystemInfo} with package scope so only the {@link
SystemInfoFactory} can instantiate
- * this object.
- */
- JavaSystemInfo() {
- }
-
private UnsupportedOperationException getUnsupportedException(Exception e) {
return new UnsupportedOperationException("No native library available -
" + e, e);
}
diff --git
a/modules/core/native-system/src/main/java/org/rhq/core/system/SystemInfoFactory.java
b/modules/core/native-system/src/main/java/org/rhq/core/system/SystemInfoFactory.java
index df35169..4c4fdd4 100644
--- a/modules/core/native-system/src/main/java/org/rhq/core/system/SystemInfoFactory.java
+++ b/modules/core/native-system/src/main/java/org/rhq/core/system/SystemInfoFactory.java
@@ -31,6 +31,10 @@ import java.net.URL;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -59,6 +63,25 @@ public class SystemInfoFactory {
private static boolean disabled;
private static boolean initialized = false;
+ private static final ThreadFactory threadFactory = new ThreadFactory() {
+ final AtomicInteger threadNumber = new AtomicInteger(1);
+
+ @Override
+ public Thread newThread(Runnable r) {
+ Thread t = new Thread(r);
+ t.setName("systeminfo-" + threadNumber.getAndIncrement());
+ return t;
+ }
+ };
+
+ /**
+ * Used for mainly process execution within the plugin container.
+ * This should be 'final' but shutdown() is not really final either.
+ */
+ private static ExecutorService executor =
Executors.newCachedThreadPool(threadFactory);
+
+ private static SystemInfo javaSystemInfo = new JavaSystemInfo(executor);
+
private static SystemInfo cachedSystemInfo;
// to avoid having this class need the native JNI classes at compile or load time,
we'll build up
@@ -203,7 +226,7 @@ public class SystemInfoFactory {
}
if (nativePlatform == null) {
- nativePlatform = new JavaSystemInfo();
+ nativePlatform = javaSystemInfo;
}
cachedSystemInfo = nativePlatform;
@@ -220,7 +243,7 @@ public class SystemInfoFactory {
* @return Java-only {@link SystemInfo} implementation
*/
public static SystemInfo createJavaSystemInfo() {
- return new JavaSystemInfo();
+ return javaSystemInfo;
}
/**
@@ -228,6 +251,13 @@ public class SystemInfoFactory {
* time it would be appropriate to give the native libraries a chance to clean up
after themselves.
*/
public static synchronized void shutdown() {
+ // Unclear if this should be 'shutdownNow()' or not
+ // This is used to execute sub processes; so it seems polite to wait for
completion
+ executor.shutdown();
+ // This is needed by tests
+ executor = Executors.newCachedThreadPool(threadFactory);
+ javaSystemInfo = new JavaSystemInfo(executor);
+
if (initialized) {
// initialized is only ever set to true if the native layer was actually
initialized
// we don't want or need to check enabled/disabled here; we could have
disabled after
@@ -402,7 +432,7 @@ public class SystemInfoFactory {
}
}
- // create a base IP address - this one is known to java and should always
exist no matter what platform we are on
+ // create a base IP address - this one is known to java and should always
exist no matter what platform we are on
try {
try {
tokens.put(TOKEN_PREFIX + "interfaces.java.address",
InetAddress
@@ -420,4 +450,5 @@ public class SystemInfoFactory {
return new TemplateEngine(new HashMap<String, String>());
}
}
+
}
diff --git
a/modules/core/native-system/src/test/java/org/rhq/core/system/NativeSystemInfoTest.java
b/modules/core/native-system/src/test/java/org/rhq/core/system/NativeSystemInfoTest.java
index 68ebebc..8a8c358 100644
---
a/modules/core/native-system/src/test/java/org/rhq/core/system/NativeSystemInfoTest.java
+++
b/modules/core/native-system/src/test/java/org/rhq/core/system/NativeSystemInfoTest.java
@@ -305,7 +305,7 @@ public class NativeSystemInfoTest {
*/
@Test(enabled = ENABLED)
public void testJava() {
- processJavaSystemInfo(new JavaSystemInfo());
+ processJavaSystemInfo(SystemInfoFactory.createJavaSystemInfo());
}
/**
diff --git a/modules/core/util/src/main/java/org/rhq/core/util/exec/ProcessExecutor.java
b/modules/core/util/src/main/java/org/rhq/core/util/exec/ProcessExecutor.java
index e74bcce..07b05f5 100644
--- a/modules/core/util/src/main/java/org/rhq/core/util/exec/ProcessExecutor.java
+++ b/modules/core/util/src/main/java/org/rhq/core/util/exec/ProcessExecutor.java
@@ -34,6 +34,7 @@ import java.io.OutputStream;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
@@ -43,7 +44,8 @@ import java.util.concurrent.TimeoutException;
import org.rhq.core.util.UtilI18NResourceKeys;
/**
- * Executes a process using 100% Java API.
+ * Executes processes using the Java API.
+ * This instance is thread safe and can be reused.
*
* <p><b>Warning: caution should be exercised when using this class - it
allows any process to be started with no
* security restrictions.</b></p>
@@ -52,7 +54,26 @@ import org.rhq.core.util.UtilI18NResourceKeys;
*/
public class ProcessExecutor {
- private static ExecutorService threadPool = Executors.newCachedThreadPool();
+ /**
+ * A thread pool for executing processes.
+ * Although it would make sense to have this 'static', it mucks up RHQ Agent
cleanup.
+ * Far better is to keep a reference to this instance.
+ */
+ private final ExecutorService threadPool;
+
+ /**
+ * Constructs with a thread pool which executes tasks.
+ */
+ public ProcessExecutor(ExecutorService threadPool) {
+ this.threadPool = threadPool;
+ }
+
+ /**
+ * Constructs using a new, cached thread pool.
+ */
+ public ProcessExecutor() {
+ this(Executors.newCachedThreadPool());
+ }
/**
* This executes any operating system process as described in the given start
command. When this method returns, it
@@ -147,10 +168,10 @@ public class ProcessExecutor {
*/
protected static class RedirectThreads {
- private final StreamRedirector stdout;
- private final StreamRedirector stderr;
+ private final Future<?> stdout;
+ private final Future<?> stderr;
- private RedirectThreads(StreamRedirector stdout, StreamRedirector stderr) {
+ private RedirectThreads(Future<?> stdout, Future<?> stderr) {
this.stdout = stdout;
this.stderr = stderr;
}
@@ -158,17 +179,17 @@ public class ProcessExecutor {
/**
* Waits for output to be fully captured.
*/
- public void join() throws InterruptedException {
- stderr.join();
- stdout.join();
+ public void join() throws InterruptedException, ExecutionException {
+ stderr.get();
+ stdout.get();
}
/**
* Interrupts these threads.
*/
public void interrupt() {
- stderr.interrupt();
- stdout.interrupt();
+ stderr.cancel(true);
+ stdout.cancel(true);
}
}
@@ -222,8 +243,8 @@ public class ProcessExecutor {
StreamRedirector stdoutThread = new StreamRedirector(threadNamePrefix +
"-stdout", stdout, fileOutputStream);
StreamRedirector stderrThread = new StreamRedirector(threadNamePrefix +
"-stderr", stderr, fileOutputStream);
- stdoutThread.start();
- stderrThread.start();
+ Future<?> out = threadPool.submit(stdoutThread);
+ Future<?> err = threadPool.submit(stderrThread);
// if an input file was specified, take the file's data and write it to the
process' stdin
File inputFile = getInputFile(process);
@@ -245,7 +266,7 @@ public class ProcessExecutor {
stdin.close();
- return new RedirectThreads(stdoutThread, stderrThread);
+ return new RedirectThreads(out, err);
}
/**
diff --git a/modules/core/util/src/main/java/org/rhq/core/util/exec/StreamRedirector.java
b/modules/core/util/src/main/java/org/rhq/core/util/exec/StreamRedirector.java
index afff92d..2ed4ee2 100644
--- a/modules/core/util/src/main/java/org/rhq/core/util/exec/StreamRedirector.java
+++ b/modules/core/util/src/main/java/org/rhq/core/util/exec/StreamRedirector.java
@@ -31,7 +31,7 @@ import java.io.OutputStream;
*
* @author John Mazzitelli
*/
-public class StreamRedirector extends Thread {
+public class StreamRedirector implements Runnable {
/**
* the stream where we read data from
*/
@@ -42,6 +42,8 @@ public class StreamRedirector extends Thread {
*/
private final OutputStream m_output;
+ private final String m_name;
+
/**
* Constructor for {@link StreamRedirector} that takes an input stream where we read
data in and an output stream
* where we write the data read from the input stream. If the output stream is
<code>null</code>, the incoming data
@@ -54,18 +56,14 @@ public class StreamRedirector extends Thread {
* @throws IllegalArgumentException if input stream is <code>null</code>
*/
public StreamRedirector(String name, InputStream is, OutputStream os) throws
IllegalArgumentException {
- super(name);
if (is == null) {
throw new IllegalArgumentException("is=null");
}
- setDaemon(true);
-
+ m_name = name;
m_input = is;
m_output = os;
-
- return;
}
/**
@@ -73,6 +71,9 @@ public class StreamRedirector extends Thread {
*/
@Override
public void run() {
+ Thread t = Thread.currentThread();
+ t.setName(m_name);
+
final int bufferSize = 4096;
byte[] buffer = new byte[bufferSize];
boolean keepGoing = true;