Fail on runtime startup errors

Instead of just logging exceptions on startup they are not handled anymore and will now prevent startup of JaCoCo runtime. This strategy avoids deferred failures due to partially initialized runtimes.
jacoco · Jul 20, 2019 · 9b5e9f8 · 9b5e9f8
1 parent 103c66f
commit 9b5e9f8
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 31 deletions.
diff --git a/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/AgentTest.java b/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/AgentTest.java
@@ -19,8 +19,8 @@
 import static org.junit.Assert.fail;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.lang.management.ManagementFactory;
+import java.util.NoSuchElementException;
 
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanServer;
@@ -105,16 +105,15 @@ public void startup_should_create_random_session_id_when_undefined()
 		assertNull(loggedException);
 	}
 
-	@Test
-	public void startup_should_log_exception() throws Exception {
-		final Exception expected = new Exception();
+	@Test(expected = NoSuchElementException.class)
+	public void startup_should_rethrow_exception() throws Exception {
 		Agent agent = new Agent(options, this) {
 			@Override
 			IAgentOutput createAgentOutput() {
 				return new IAgentOutput() {
 					public void startup(AgentOptions options, RuntimeData data)
 							throws Exception {
-						throw expected;
+						throw new NoSuchElementException();
 					}
 
 					public void shutdown() {
@@ -127,8 +126,6 @@ public void writeExecutionData(boolean reset) {
 		};
 
 		agent.startup();
-
-		assertSame(expected, loggedException);
 	}
 
 	@Test
@@ -240,7 +237,7 @@ public void getVersion_should_return_current_version() {
 	}
 
 	@Test
-	public void getSessionId_should_return_session_id() throws IOException {
+	public void getSessionId_should_return_session_id() throws Exception {
 		Agent agent = createAgent();
 
 		agent.startup();
@@ -249,7 +246,7 @@ public void getSessionId_should_return_session_id() throws IOException {
 	}
 
 	@Test
-	public void setSessionId_should_modify_session_id() throws IOException {
+	public void setSessionId_should_modify_session_id() throws Exception {
 		Agent agent = createAgent();
 		agent.startup();
 
@@ -273,7 +270,7 @@ public void reset_should_reset_probes() {
 
 	@Test
 	public void getExecutionData_should_return_probes_and_session_id()
-			throws IOException {
+			throws Exception {
 		Agent agent = createAgent();
 		agent.startup();
 		agent.getData().getExecutionData(Long.valueOf(0x12345678), "Foo", 1)
@@ -291,7 +288,7 @@ public void getExecutionData_should_return_probes_and_session_id()
 
 	@Test
 	public void getExecutionData_should_reset_probes_when_enabled()
-			throws IOException {
+			throws Exception {
 		Agent agent = createAgent();
 		agent.startup();
 		final boolean[] probes = agent.getData()
@@ -306,7 +303,7 @@ public void getExecutionData_should_reset_probes_when_enabled()
 
 	@Test
 	public void getExecutionData_should_not_reset_probes_when_disabled()
-			throws IOException {
+			throws Exception {
 		Agent agent = createAgent();
 		agent.startup();
 		final boolean[] probes = agent.getData()

diff --git a/org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Agent.java b/org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Agent.java
@@ -43,8 +43,11 @@ public class Agent implements IAgent {
 	 * @param options
 	 *            options to configure the instance
 	 * @return global instance
+	 * @throws Exception
+	 *             in case something cannot be initialized
 	 */
-	public static synchronized Agent getInstance(final AgentOptions options) {
+	public static synchronized Agent getInstance(final AgentOptions options)
+			throws Exception {
 		if (singleton == null) {
 			final Agent agent = new Agent(options, IExceptionLogger.SYSTEM_ERR);
 			agent.startup();
@@ -67,7 +70,8 @@ public void run() {
 	 * @throws IllegalStateException
 	 *             if no Agent has been started yet
 	 */
-	public static synchronized Agent getInstance() throws IllegalStateException {
+	public static synchronized Agent getInstance()
+			throws IllegalStateException {
 		if (singleton == null) {
 			throw new IllegalStateException("JaCoCo agent not started.");
 		}
@@ -110,21 +114,19 @@ public RuntimeData getData() {
 	/**
 	 * Initializes this agent.
 	 * 
+	 * @throws Exception
+	 *             in case something cannot be initialized
 	 */
-	public void startup() {
-		try {
-			String sessionId = options.getSessionId();
-			if (sessionId == null) {
-				sessionId = createSessionId();
-			}
-			data.setSessionId(sessionId);
-			output = createAgentOutput();
-			output.startup(options, data);
-			if (options.getJmx()) {
-				jmxRegistration = new JmxRegistration(this);
-			}
-		} catch (final Exception e) {
-			logger.logExeption(e);
+	public void startup() throws Exception {
+		String sessionId = options.getSessionId();
+		if (sessionId == null) {
+			sessionId = createSessionId();
+		}
+		data.setSessionId(sessionId);
+		output = createAgentOutput();
+		output.startup(options, data);
+		if (options.getJmx()) {
+			jmxRegistration = new JmxRegistration(this);
 		}
 	}
 

diff --git a/org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Offline.java b/org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Offline.java
@@ -28,7 +28,11 @@ public final class Offline {
 	static {
 		final Properties config = ConfigLoader.load(CONFIG_RESOURCE,
 				System.getProperties());
-		DATA = Agent.getInstance(new AgentOptions(config)).getData();
+		try {
+			DATA = Agent.getInstance(new AgentOptions(config)).getData();
+		} catch (final Exception e) {
+			throw new RuntimeException("Failed to initialize JaCoCo.", e);
+		}
 	}
 
 	private Offline() {
@@ -48,8 +52,9 @@ private Offline() {
 	 */
 	public static boolean[] getProbes(final long classid,
 			final String classname, final int probecount) {
-		return DATA.getExecutionData(Long.valueOf(classid), classname,
-				probecount).getProbes();
+		return DATA
+				.getExecutionData(Long.valueOf(classid), classname, probecount)
+				.getProbes();
 	}
 
 }
diff --git a/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml b/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml
@@ -123,5 +123,24 @@
 		</java>
 		<au:assertFileExists file="${temp.dir}/test.exec" />
 	</target>
+
+	<target name="testInstrumentWithRuntimeStartupFailure">
+		<jacoco:instrument destdir="${temp.dir}">
+			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
+		</jacoco:instrument>
+		<au:assertLogContains text="Instrumented 15 classes to ${temp.dir}"/>
+		<au:assertFileExists file="${temp.dir}/org/jacoco/ant/InstrumentTaskTest.class" />
+
+		<java classname="org.jacoco.ant.TestTarget" failonerror="false" fork="true">
+			<sysproperty key="jacoco-agent.output" value="tcpserver"/>
+			<sysproperty key="jacoco-agent.port" value="foo"/>
+			<classpath>
+				<pathelement path="${org.jacoco.ant.instrumentTaskTest.agent.file}"/>
+				<pathelement path="${temp.dir}"/>
+			</classpath>
+		</java>
+		<au:assertLogContains text="Caused by: java.lang.RuntimeException: Failed to initialize JaCoCo."/>
+		<au:assertLogContains text="Caused by: java.lang.NumberFormatException: For input string: &quot;foo&quot;"/>
+	</target>
 
 </project>
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
@@ -38,6 +38,13 @@ <h3>Fixed bugs</h3>
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/894">#894</a>).</li>
 </ul>
 
+<h3>Non-functional Changes</h3>
+<ul>
+  <li>Prevent startup when JaCoCo runtime cannot be initialized to avoid
+      subsequent faults
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/910">#910</a>).</li>
+</ul>
+
 <h2>Release 0.8.4 (2019/05/08)</h2>
 
 <h3>New Features</h3>