From 008297c01720142798feb575d7caa09b5f0db485 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 12 Sep 2014 14:51:23 +0200 Subject: [PATCH] SONAR-4898 add timeout to RMI calls on isReady() --- .../sonar/process/monitor/JmxConnector.java | 15 ++- .../org/sonar/process/monitor/Monitor.java | 6 +- .../process/monitor/RmiJmxConnector.java | 97 +++++++++---------- .../process/monitor/TerminatorThread.java | 16 +-- .../monitor/CallVerifierJmxConnector.java | 4 - .../ImpossibleToConnectJmxConnector.java | 6 +- .../InfiniteTerminationRmiConnector.java | 38 -------- .../sonar/process/monitor/MonitorTest.java | 21 +--- .../process/monitor/RmiJmxConnectorTest.java | 56 +++++++++++ .../TerminationFailureRmiConnector.java | 6 +- 10 files changed, 127 insertions(+), 138 deletions(-) delete mode 100644 server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/InfiniteTerminationRmiConnector.java create mode 100644 server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JmxConnector.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JmxConnector.java index b06ea684d41..c996595ba73 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JmxConnector.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JmxConnector.java @@ -24,12 +24,21 @@ package org.sonar.process.monitor; */ public interface JmxConnector { - void connect(JavaCommand command, ProcessRef processRef); + /** + * Throws an exception if timeout reached + */ + void connect(JavaCommand command, ProcessRef processRef, long timeoutMs); void ping(ProcessRef process); - boolean isReady(ProcessRef process); + /** + * Throws an exception if timeout reached + */ + boolean isReady(ProcessRef process, long timeoutMs); - void terminate(ProcessRef process); + /** + * Throws an exception if timeout reached + */ + void terminate(ProcessRef process, long timeoutMs); } diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java index badc63b6da5..518d2c354cd 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java @@ -53,7 +53,7 @@ public class Monitor { public static Monitor create() { Timeouts timeouts = new Timeouts(); - return new Monitor(new JavaProcessLauncher(timeouts), new RmiJmxConnector(timeouts), + return new Monitor(new JavaProcessLauncher(timeouts), new RmiJmxConnector(), timeouts, new SystemExit()); } @@ -104,7 +104,7 @@ public class Monitor { watcherThreads.add(watcherThread); // add to list of monitored processes only when successfully connected to it - jmxConnector.connect(command, processRef); + jmxConnector.connect(command, processRef, timeouts.getJmxConnectionTimeout()); processes.add(processRef); // ping process on a regular basis @@ -126,7 +126,7 @@ public class Monitor { throw new MessageException(String.format("%s failed to start", processRef)); } try { - ready = jmxConnector.isReady(processRef); + ready = jmxConnector.isReady(processRef, timeouts.getMonitorIsReadyTimeout()); } catch (Exception ignored) { // failed to send request, probably because RMI server is still not alive // trying again, as long as process is alive diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java index 2ca0a295b87..ec8dba2902a 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java @@ -43,45 +43,32 @@ class RmiJmxConnector implements JmxConnector { static { /* - Prevents such warnings : - - WARNING: Failed to restart: java.io.IOException: Failed to get a RMI stub: javax.naming.ServiceUnavailableException [Root exception is java.rmi.ConnectException: Connection refused to host: 127.0.0.1; nested exception is: - java.net.ConnectException: Connection refused] - Sep 11, 2014 7:32:32 PM RMIConnector RMIClientCommunicatorAdmin-doStop - WARNING: Failed to call the method close():java.rmi.ConnectException: Connection refused to host: 127.0.0.1; nested exception is: - java.net.ConnectException: Connection refused - Sep 11, 2014 7:32:32 PM ClientCommunicatorAdmin Checker-run - WARNING: Failed to check connection: java.net.ConnectException: Connection refused - Sep 11, 2014 7:32:32 PM ClientCommunicatorAdmin Checker-run - WARNING: stopping + * Prevents such warnings : + * + * WARNING: Failed to restart: java.io.IOException: Failed to get a RMI stub: javax.naming.ServiceUnavailableException [Root exception + * is java.rmi.ConnectException: Connection refused to host: 127.0.0.1; nested exception is: + * java.net.ConnectException: Connection refused] + * Sep 11, 2014 7:32:32 PM RMIConnector RMIClientCommunicatorAdmin-doStop + * WARNING: Failed to call the method close():java.rmi.ConnectException: Connection refused to host: 127.0.0.1; nested exception is: + * java.net.ConnectException: Connection refused + * Sep 11, 2014 7:32:32 PM ClientCommunicatorAdmin Checker-run + * WARNING: Failed to check connection: java.net.ConnectException: Connection refused + * Sep 11, 2014 7:32:32 PM ClientCommunicatorAdmin Checker-run + * WARNING: stopping */ System.setProperty("sun.rmi.transport.tcp.logLevel", "SEVERE"); } private final Map mbeans = new IdentityHashMap(); - private final Timeouts timeouts; - - RmiJmxConnector(Timeouts timeouts) { - this.timeouts = timeouts; - } @Override - public synchronized void connect(final JavaCommand command, ProcessRef processRef) { - ExecutorService executor = Executors.newSingleThreadExecutor(); + public synchronized void connect(final JavaCommand command, ProcessRef processRef, long timeoutMs) { ConnectorCallable callable = new ConnectorCallable(command, processRef.getProcess()); - try { - Future future = executor.submit(callable); - ProcessMXBean mxBean = future.get(timeouts.getJmxConnectionTimeout(), TimeUnit.MILLISECONDS); - if (mxBean != null) { - mbeans.put(processRef, mxBean); - } - } catch (Exception e) { - if (callable.latestException != null) { - throw callable.latestException; - } - throw new IllegalStateException("Fail to connect to JMX", e); - } finally { - executor.shutdownNow(); + ProcessMXBean mxBean = execute(callable, timeoutMs); + if (mxBean != null) { + register(processRef, mxBean); + } else if (!processRef.isTerminated()) { + throw new IllegalStateException("Fail to connect to JMX RMI server of " + processRef); } } @@ -91,32 +78,45 @@ class RmiJmxConnector implements JmxConnector { } @Override - public boolean isReady(final ProcessRef processRef) { + public boolean isReady(final ProcessRef processRef, long timeoutMs) { + return execute(new Callable() { + @Override + public Boolean call() throws Exception { + return mbeans.get(processRef).isReady(); + } + }, timeoutMs); + } + + @Override + public void terminate(final ProcessRef processRef, long timeoutMs) { + execute(new Callable() { + @Override + public Void call() throws Exception { + mbeans.get(processRef).terminate(); + return null; + } + }, timeoutMs); + } + + void register(ProcessRef processRef, ProcessMXBean mxBean) { + mbeans.put(processRef, mxBean); + } + + private T execute(Callable callable, long timeoutMs) { ExecutorService executor = Executors.newSingleThreadExecutor(); try { - Future future = executor.submit(new Callable() { - @Override - public Boolean call() throws Exception { - return mbeans.get(processRef).isReady(); - } - }); - return future.get(timeouts.getMonitorIsReadyTimeout(), TimeUnit.MILLISECONDS); + Future future = executor.submit(callable); + return future.get(timeoutMs, TimeUnit.MILLISECONDS); } catch (Exception e) { - throw new IllegalStateException("Fail send JMX request (isReady)", e); + throw new IllegalStateException("Fail send JMX request", e); } finally { executor.shutdownNow(); } } - @Override - public void terminate(ProcessRef processRef) { - mbeans.get(processRef).terminate(); - } - private static class ConnectorCallable implements Callable { private final JavaCommand command; private final Process process; - private RuntimeException latestException; private ConnectorCallable(JavaCommand command, Process process) { this.command = command; @@ -132,9 +132,8 @@ class RmiJmxConnector implements JmxConnector { JMXConnector jmxConnector = JMXConnectorFactory.connect(jmxUrl, null); MBeanServerConnection mBeanServer = jmxConnector.getMBeanServerConnection(); return JMX.newMBeanProxy(mBeanServer, JmxUtils.objectName(command.getKey()), ProcessMXBean.class); - } catch (Exception e) { - latestException = new IllegalStateException(String.format( - "Fail to connect to JMX bean of %s [%s] ", command.getKey(), jmxUrl), e); + } catch (Exception ignored) { + // ignored, RMI server is probably not started yet } Thread.sleep(300L); } diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/TerminatorThread.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/TerminatorThread.java index 775a036bb2f..f0943086877 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/TerminatorThread.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/TerminatorThread.java @@ -22,10 +22,6 @@ package org.sonar.process.monitor; import org.slf4j.LoggerFactory; import java.util.List; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; /** * Terminates all monitored processes. Tries to gracefully terminate each process, @@ -53,22 +49,12 @@ class TerminatorThread extends Thread { if (!processRef.isTerminated()) { processRef.setPingEnabled(false); - ExecutorService executor = Executors.newSingleThreadExecutor(); - Future future = executor.submit(new Runnable() { - @Override - public void run() { - // ask for graceful termination - LoggerFactory.getLogger(getClass()).info("Request termination of " + processRef); - jmxConnector.terminate(processRef); - } - }); try { - future.get(timeouts.getTerminationTimeout(), TimeUnit.MILLISECONDS); + jmxConnector.terminate(processRef, timeouts.getTerminationTimeout()); } catch (Exception ignored) { // failed to gracefully stop in a timely fashion LoggerFactory.getLogger(getClass()).info(String.format("Kill %s", processRef)); } finally { - executor.shutdownNow(); // kill even if graceful termination was done, just to be sure that physical process is really down processRef.hardKill(); } diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/CallVerifierJmxConnector.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/CallVerifierJmxConnector.java index 3c0c7be0208..2010e19aa87 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/CallVerifierJmxConnector.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/CallVerifierJmxConnector.java @@ -26,10 +26,6 @@ public class CallVerifierJmxConnector extends RmiJmxConnector { boolean askedPing = false; - CallVerifierJmxConnector(Timeouts timeouts) { - super(timeouts); - } - @Override public void ping(ProcessRef process) { askedPing = true; diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/ImpossibleToConnectJmxConnector.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/ImpossibleToConnectJmxConnector.java index 0df9ee9f023..055e3c8e50b 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/ImpossibleToConnectJmxConnector.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/ImpossibleToConnectJmxConnector.java @@ -21,7 +21,7 @@ package org.sonar.process.monitor; public class ImpossibleToConnectJmxConnector implements JmxConnector { @Override - public void connect(JavaCommand command, ProcessRef processRef) { + public void connect(JavaCommand command, ProcessRef processRef, long timeoutMs) { throw new IllegalStateException("Test - Impossible to connect to JMX"); } @@ -31,12 +31,12 @@ public class ImpossibleToConnectJmxConnector implements JmxConnector { } @Override - public boolean isReady(ProcessRef process) { + public boolean isReady(ProcessRef process, long timeoutMs) { return false; } @Override - public void terminate(ProcessRef process) { + public void terminate(ProcessRef process, long timeoutMs) { } } diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/InfiniteTerminationRmiConnector.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/InfiniteTerminationRmiConnector.java deleted file mode 100644 index b252379ae8d..00000000000 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/InfiniteTerminationRmiConnector.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.process.monitor; - -public class InfiniteTerminationRmiConnector extends RmiJmxConnector { - - InfiniteTerminationRmiConnector(Timeouts timeouts) { - super(timeouts); - } - - @Override - public void terminate(ProcessRef processRef) { - try { - while (true) { - Thread.sleep(50L); - } - } catch (Exception e) { - - } - } -} diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java index 291b91fbad4..6f0be0eb5be 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java @@ -226,27 +226,12 @@ public class MonitorTest { } } - @Test - public void kill_process_if_too_long_to_request_gracefully_termination() throws Exception { - Timeouts timeouts = new Timeouts(); - timeouts.setTerminationTimeout(100L); - monitor = new Monitor(new JavaProcessLauncher(timeouts), - new InfiniteTerminationRmiConnector(timeouts), timeouts, exit); - - HttpProcessClient p1 = new HttpProcessClient("p1"); - monitor.start(Arrays.asList(p1.newCommand())); - assertThat(p1.isReady()).isTrue(); - - monitor.stop(); - assertThat(p1.isReady()).isFalse(); - } - @Test public void kill_process_if_fail_to_request_gracefully_termination() throws Exception { Timeouts timeouts = new Timeouts(); timeouts.setTerminationTimeout(100L); monitor = new Monitor(new JavaProcessLauncher(timeouts), - new TerminationFailureRmiConnector(timeouts), timeouts, exit); + new TerminationFailureRmiConnector(), timeouts, exit); HttpProcessClient p1 = new HttpProcessClient("p1"); monitor.start(Arrays.asList(p1.newCommand())); @@ -315,7 +300,7 @@ public class MonitorTest { timeouts.setMonitorPingInterval(10L); timeouts.setAutokillPingInterval(10L); timeouts.setAutokillPingTimeout(10L); - CallVerifierJmxConnector jmxConnector = new CallVerifierJmxConnector(timeouts); + CallVerifierJmxConnector jmxConnector = new CallVerifierJmxConnector(); monitor = new Monitor(new JavaProcessLauncher(timeouts), jmxConnector, timeouts, exit); JavaCommand command = newStandardProcessCommand() @@ -330,7 +315,7 @@ public class MonitorTest { private Monitor newDefaultMonitor() { Timeouts timeouts = new Timeouts(); - return new Monitor(new JavaProcessLauncher(timeouts), new RmiJmxConnector(timeouts), timeouts, exit); + return new Monitor(new JavaProcessLauncher(timeouts), new RmiJmxConnector(), timeouts, exit); } /** diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java new file mode 100644 index 00000000000..c4a0f735d60 --- /dev/null +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java @@ -0,0 +1,56 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.process.monitor; + +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.sonar.process.ProcessMXBean; + +import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RmiJmxConnectorTest { + + @Test + public void throw_exception_on_timeout() throws Exception { + RmiJmxConnector connector = new RmiJmxConnector(); + ProcessRef ref = mock(ProcessRef.class); + ProcessMXBean mxBean = mock(ProcessMXBean.class); + connector.register(ref, mxBean); + + when(mxBean.isReady()).thenAnswer(new Answer() { + @Override + public Boolean answer(InvocationOnMock invocationOnMock) throws Throwable { + Thread.sleep(Long.MAX_VALUE); + return null; + } + }); + + try { + connector.isReady(ref, 5L); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessage("Fail send JMX request"); + } + } +} diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TerminationFailureRmiConnector.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TerminationFailureRmiConnector.java index 9fb9405f21d..0f9f5702666 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TerminationFailureRmiConnector.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TerminationFailureRmiConnector.java @@ -20,12 +20,8 @@ package org.sonar.process.monitor; public class TerminationFailureRmiConnector extends RmiJmxConnector { - TerminationFailureRmiConnector(Timeouts timeouts) { - super(timeouts); - } - @Override - public void terminate(ProcessRef processRef) { + public void terminate(ProcessRef processRef, long timeoutMs) { throw new IllegalStateException("Test - fail to send termination request"); } } -- 2.39.5