Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent startup when runtime cannot be initialized #910

Merged
merged 4 commits into from Sep 19, 2019
Merged

Prevent startup when runtime cannot be initialized #910

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b5e9f8
Fail on runtime startup errors
marchof Jul 19, 2019
bd40de1
Initialize Offline runtime outside class initializers
marchof Aug 6, 2019
3a4fe11
Still log startup exception
marchof Sep 19, 2019
f8e6a6f
Merge branch 'master' into fail_on_startup
Godin Sep 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 9 additions & 12 deletions org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/AgentTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -127,8 +126,6 @@ public void writeExecutionData(boolean reset) {
};

agent.startup();

assertSame(expected, loggedException);
}

@Test
Expand Down Expand Up @@ -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();
Expand All @@ -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();

Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand Down
34 changes: 18 additions & 16 deletions org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Agent.java
View file
Open in desktop
Expand Up @@ -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();
Expand All @@ -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.");
}
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marchof maybe we should keep this output in addition to throwing of exception for the case when caller swallows exception, e.g. offline instrumented classes of app inside of app-server? WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Godin Perfectly valid point. I restored logging.

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);
}
}

Expand Down
27 changes: 18 additions & 9 deletions org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/Offline.java
View file
Open in desktop
Expand Up @@ -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.
*
Expand All @@ -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();
}

}
19 changes: 19 additions & 0 deletions org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml
View file
Open in desktop
Expand Up @@ -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>
7 changes: 7 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
View file
Open in desktop
Expand Up @@ -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>
Expand Down