Prevent startup when runtime cannot be initialized (#910)

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 · Sep 19, 2019 · 40c8fd8 · 40c8fd8
1 parent 768968d
commit 40c8fd8
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 22 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,7 +19,6 @@
 import static org.junit.Assert.fail;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.lang.management.ManagementFactory;
 
 import javax.management.InstanceNotFoundException;
@@ -106,8 +105,9 @@ public void startup_should_create_random_session_id_when_undefined()
 	}
 
 	@Test
-	public void startup_should_log_exception() throws Exception {
+	public void startup_should_log_and_rethrow_exception() throws Exception {
 		final Exception expected = new Exception();
+
 		Agent agent = new Agent(options, this) {
 			@Override
 			IAgentOutput createAgentOutput() {
@@ -126,9 +126,13 @@ public void writeExecutionData(boolean reset) {
 			}
 		};
 
-		agent.startup();
-
-		assertSame(expected, loggedException);
+		try {
+			agent.startup();
+			fail("Exception expected");
+		} catch (Exception actual) {
+			assertSame(expected, actual);
+			assertSame(expected, loggedException);
+		}
 	}
 
 	@Test
@@ -240,7 +244,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 +253,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 +277,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 +295,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 +310,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,8 +114,10 @@ public RuntimeData getData() {
 	/**
 	 * Initializes this agent.
 	 * 
+	 * @throws Exception
+	 *             in case something cannot be initialized
 	 */
-	public void startup() {
+	public void startup() throws Exception {
 		try {
 			String sessionId = options.getSessionId();
 			if (sessionId == null) {
@@ -125,6 +131,7 @@ public void startup() {
 			}
 		} catch (final Exception e) {
 			logger.logExeption(e);
+			throw e;
 		}
 	}
 

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
@@ -22,19 +22,27 @@
  */
 public final class Offline {
 
-	private static final RuntimeData DATA;
 	private static final String CONFIG_RESOURCE = "/jacoco-agent.properties";
 
-	static {
-		final Properties config = ConfigLoader.load(CONFIG_RESOURCE,
-				System.getProperties());
-		DATA = Agent.getInstance(new AgentOptions(config)).getData();
-	}
-
 	private Offline() {
 		// no instances
 	}
 
+	private static RuntimeData data;
+
+	private static synchronized RuntimeData getRuntimeData() {
+		if (data == null) {
+			final Properties config = ConfigLoader.load(CONFIG_RESOURCE,
+					System.getProperties());
+			try {
+				data = Agent.getInstance(new AgentOptions(config)).getData();
+			} catch (final Exception e) {
+				throw new RuntimeException("Failed to initialize JaCoCo.", e);
+			}
+		}
+		return data;
+	}
+
 	/**
 	 * API for offline instrumented classes.
 	 * 
@@ -48,8 +56,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 getRuntimeData()
+				.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="java.lang.RuntimeException: Failed to initialize JaCoCo."/>
+		<au:assertLogContains text="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
@@ -46,6 +46,13 @@ <h3>Fixed bugs</h3>
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/912">#912</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>