Description of problem: If org.rhq.core.util.exec.ProcessExecutor() is used in a plugin, then Java will keep around an idle thread for a bit, like 60 seconds. This slows down the agent shutdown process. It might make sense to create a "ProcessExecutor.shutdown()" method that is called by the agent container that will free this thread. Or another approach is recommended. 2012-11-02 19:35:32,928 DEBUG [Thread-7] (org.rhq.enterprise.agent.AgentShutdownHook)- {AgentShutdownHook.thread-is-still-active}Thread [ExecuteProcess-/usr/local/bin/foo] is still alive - its stack trace follows: java.lang.Throwable: Thread [ExecuteProcess-/usr/local/bin/foo] at sun.misc.Unsafe.park(Native Method) at java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:226) at java.util.concurrent.SynchronousQueue$TransferStack.awaitFulfill(SynchronousQueue.java:453) at java.util.concurrent.SynchronousQueue$TransferStack.transfer(SynchronousQueue.java:352) at java.util.concurrent.SynchronousQueue.poll(SynchronousQueue.java:903) at java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1043) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1103) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) at java.lang.Thread.run(Thread.java:636)
Or try setting the thread to a daemon one. Although you may not really want that thread terminated if the agent is running some sort of shell command. You can also set the timeout for pooled threads to be a lot smaller than 60 seconds. Or remove the 'static' declaration.
Created attachment 639678 [details] incomplete patch, but addresses some of the design The easiest fix is probably okay, but references to ProcessExecutor should probably be retained otherwise a thread pool is created on every exec. I changed the classes to hold a reference to a ProcessExecutor. But still the issue is the Agent container should really be responsible and shut down the exec thread pool. How can this hook be created? Could ResourceContainer's thread pool be used? Etc. Another issue if the stream I/O threads are really daemon or not.
Created attachment 763100 [details] This is the complete suggested fix The only complaint I have is, is that SystemInfoFactory.shutdown() should really be a final shutdown.
In org.rhq.core.util.exec.StreamRedirector.StreamRedirector(String, InputStream, OutputStream), I see this has now been removed (because it is no longer extending Thread, but now only implementing Runnable): setDaemon(true); However, I don't see the threads created by the thread factory to be daemon threads (the default, I believe, is to be non-daemon). In order to get back to the original behavior, I think we need to add: t.setDaemon(true); to: org.rhq.core.system.SystemInfoFactory.threadFactory.new ThreadFactory() {...}.newThread(Runnable) after line 72: public Thread newThread(Runnable r) { Thread t = new Thread(r); t.setName("systeminfo-" + threadNumber.getAndIncrement()); t.setDaemon(true); return t; } However, I am not going to do this now because I think the StreamRedirector threads are getting created by the same executor/thread pool that is running everything else (which I don't think we want to be daemon). The question becomes - what happens if these stream redirect threads hang? It used to be it wouldn't matter - the agent VM will still go down because these were daemon threads. But now they are not. Is is possible this new shutdown code could leave some non-daemon threads hanging around?
(In reply to Elias Ross from comment #3) > Created attachment 763100 [details] > This is the complete suggested fix > > The only complaint I have is, is that SystemInfoFactory.shutdown() should > really be a final shutdown. I pushed this to master branch - git commit : b361a1b07c7d14834fcf4986e09f7bf683be4bf8
the patch has been pushed to master and thus will be in RHQ 4.9. Moving this to ON_QA
Bulk closing now that 4.10 is out. If you think an issue is not resolved, please open a new BZ and link to the existing one.