Skip to content

Commit

Permalink
Improve support for multiple JaCoCo runtimes in the same VM (#1057)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marchof
marchof committed Apr 4, 2022
1 parent 7e516b1 commit aa16a7c
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 28 deletions.
4 changes: 4 additions & 0 deletions org.jacoco.agent.rt.test/pom.xml
View file
Expand Up @@ -33,6 +33,10 @@
<groupId>${project.groupId}</groupId>
<artifactId>org.jacoco.agent.rt</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>org.jacoco.core.test</artifactId>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down
73 changes: 67 additions & 6 deletions org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/output/FileOutputTest.java
View file
Expand Up @@ -14,12 +14,18 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.nio.channels.OverlappingFileLockException;

import org.jacoco.core.runtime.AgentOptions;
import org.jacoco.core.runtime.RuntimeData;
import org.jacoco.core.test.validation.JavaVersion;
import org.junit.AssumptionViolatedException;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand All @@ -33,7 +39,7 @@ public class FileOutputTest {
public TemporaryFolder folder = new TemporaryFolder();

@Test
public void testCreateDestFileOnStartup() throws Exception {
public void startup_should_create_empty_execfile() throws Exception {
File destFile = folder.newFile("jacoco.exec");
AgentOptions options = new AgentOptions();
options.setDestfile(destFile.getAbsolutePath());
Expand All @@ -47,7 +53,7 @@ public void testCreateDestFileOnStartup() throws Exception {
}

@Test
public void testWriteData() throws Exception {
public void writeExecutionData_should_write_execdata() throws Exception {
File destFile = folder.newFile("jacoco.exec");
AgentOptions options = new AgentOptions();
options.setDestfile(destFile.getAbsolutePath());
Expand All @@ -62,14 +68,69 @@ public void testWriteData() throws Exception {
destFile.length() > 0);
}

@Test(expected = IOException.class)
public void testInvalidDestFile() throws Exception {
@Test
public void startup_should_throw_IOException_when_execfile_cannot_be_created()
throws Exception {
AgentOptions options = new AgentOptions();
options.setDestfile(folder.newFolder("folder").getAbsolutePath());
FileOutput controller = new FileOutput();

// Startup should fail as the file can not be created:
controller.startup(options, new RuntimeData());
try {
controller.startup(options, new RuntimeData());
fail("IOException expected");
} catch (IOException e) {
// expected
}
}

@Test
public void startup_should_throw_OverlappingFileLockException_when_execfile_is_permanently_locked()
throws Exception {
if (JavaVersion.current().isBefore("1.6")) {
throw new AssumptionViolatedException(
"OverlappingFileLockException only thrown since Java 1.6");
}

File destFile = folder.newFile("jacoco.exec");
AgentOptions options = new AgentOptions();
options.setDestfile(destFile.getAbsolutePath());
FileOutputStream out = new FileOutputStream(destFile);
out.getChannel().lock();
FileOutput controller = new FileOutput();

try {
controller.startup(options, new RuntimeData());
fail("OverlappingFileLockException expected");
} catch (OverlappingFileLockException e) {
// expected
} finally {
out.close();
}
}

public void startup_should_throw_InterruptedIOException_when_execfile_is_locked_and_thread_is_interrupted()
throws Exception {
if (JavaVersion.current().isBefore("1.6")) {
throw new AssumptionViolatedException(
"OverlappingFileLockException only thrown since Java 1.6");
}

File destFile = folder.newFile("jacoco.exec");
AgentOptions options = new AgentOptions();
options.setDestfile(destFile.getAbsolutePath());
FileOutputStream out = new FileOutputStream(destFile);
out.getChannel().lock();
FileOutput controller = new FileOutput();
Thread.currentThread().interrupt();

try {
controller.startup(options, new RuntimeData());
fail("InterruptedIOException expected");
} catch (InterruptedIOException e) {
// expected
} finally {
out.close();
}
}

}
31 changes: 29 additions & 2 deletions org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/output/FileOutput.java
View file
Expand Up @@ -15,7 +15,10 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.io.OutputStream;
import java.nio.channels.FileChannel;
import java.nio.channels.OverlappingFileLockException;

import org.jacoco.core.data.ExecutionDataWriter;
import org.jacoco.core.runtime.AgentOptions;
Expand All @@ -31,6 +34,10 @@
*/
public class FileOutput implements IAgentOutput {

private static final int LOCK_RETRY_COUNT = 30;

private static final long LOCK_RETRY_WAIT_TIME_MS = 100;

private RuntimeData data;

private File destFile;
Expand Down Expand Up @@ -67,8 +74,28 @@ public void shutdown() throws IOException {
private OutputStream openFile() throws IOException {
final FileOutputStream file = new FileOutputStream(destFile, append);
// Avoid concurrent writes from different agents running in parallel:
file.getChannel().lock();
return file;
final FileChannel fc = file.getChannel();
int retries = 0;
while (true) {
try {
// An agent from another JVM might have a lock. In this case
// this method blocks until the lock is freed.
fc.lock();
return file;
} catch (final OverlappingFileLockException e) {
// In the case of multiple class loaders there can be multiple
// JaCoCo runtimes even in the same VM. In this case we get an
// OverlappingFileLockException and retry lock acquisition:
if (retries++ > LOCK_RETRY_COUNT) {
throw e;
}
}
try {
Thread.sleep(LOCK_RETRY_WAIT_TIME_MS);
} catch (final InterruptedException e) {
throw new InterruptedIOException();
}
}
}

}
3 changes: 2 additions & 1 deletion ....core.test.validation.java5/src/org/jacoco/core/test/validation/java5/EnumSwitchTest.java
View file
Expand Up @@ -12,6 +12,7 @@
*******************************************************************************/
package org.jacoco.core.test.validation.java5;

import org.jacoco.core.test.validation.JavaVersion;
import org.jacoco.core.test.validation.Source.Line;
import org.jacoco.core.test.validation.ValidationTestBase;
import org.jacoco.core.test.validation.java5.targets.EnumSwitchTarget;
Expand All @@ -27,7 +28,7 @@ public EnumSwitchTest() {
}

public void assertSwitch(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.6")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.6")) {
// class that holds "switch map" is not marked as synthetic when
// compiling with javac 1.5
assertPartlyCovered(line, 0, 2);
Expand Down
15 changes: 8 additions & 7 deletions ...oco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/FinallyTest.java
View file
Expand Up @@ -24,6 +24,7 @@

import org.jacoco.core.internal.instr.InstrSupport;
import org.jacoco.core.test.TargetLoader;
import org.jacoco.core.test.validation.JavaVersion;
import org.jacoco.core.test.validation.Source.Line;
import org.jacoco.core.test.validation.ValidationTestBase;
import org.jacoco.core.test.validation.java5.targets.FinallyTarget;
Expand Down Expand Up @@ -62,7 +63,7 @@ public void assertFinally(final Line line) {
}

public void assertTwoRegions1(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
// https://bugs.openjdk.java.net/browse/JDK-7008643
assertPartlyCovered(line);
} else {
Expand All @@ -71,7 +72,7 @@ public void assertTwoRegions1(final Line line) {
}

public void assertTwoRegionsReturn1(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
// https://bugs.openjdk.java.net/browse/JDK-7008643
assertEmpty(line);
} else {
Expand All @@ -80,7 +81,7 @@ public void assertTwoRegionsReturn1(final Line line) {
}

public void assertTwoRegionsReturn2(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
// https://bugs.openjdk.java.net/browse/JDK-7008643
assertEmpty(line);
} else {
Expand All @@ -89,7 +90,7 @@ public void assertTwoRegionsReturn2(final Line line) {
}

public void assertEmptyTry1(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
// compiler bug fixed in javac >= 1.8:
assertPartlyCovered(line);
} else {
Expand All @@ -98,7 +99,7 @@ public void assertEmptyTry1(final Line line) {
}

public void assertEmptyTry2(final Line line) {
if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
// compiler bug fixed in javac >= 1.8:
assertFullyCovered(line);
} else {
Expand Down Expand Up @@ -146,7 +147,7 @@ private void gotos() throws IOException {

expected.add("breakStatement.for");
if (isJDKCompiler) {
if (JAVA_VERSION.isBefore("10")) {
if (JavaVersion.current().isBefore("10")) {
// https://bugs.openjdk.java.net/browse/JDK-8180141
expected.add("breakStatement.1");
} else {
Expand Down Expand Up @@ -179,7 +180,7 @@ private void gotos() throws IOException {
expected.add("nested.3");
}

if (isJDKCompiler && JAVA_VERSION.isBefore("1.8")) {
if (isJDKCompiler && JavaVersion.current().isBefore("1.8")) {
expected.add("emptyTry.2");
}

Expand Down
13 changes: 7 additions & 6 deletions ...test.validation.java7/src/org/jacoco/core/test/validation/java7/TryWithResourcesTest.java
View file
Expand Up @@ -12,6 +12,7 @@
*******************************************************************************/
package org.jacoco.core.test.validation.java7;

import org.jacoco.core.test.validation.JavaVersion;
import org.jacoco.core.test.validation.Source.Line;
import org.jacoco.core.test.validation.ValidationTestBase;
import org.jacoco.core.test.validation.java7.targets.TryWithResourcesTarget;
Expand All @@ -28,7 +29,7 @@ public TryWithResourcesTest() {

public void assertTry(final Line line) {
// without filter this line is covered partly:
if (!isJDKCompiler || JAVA_VERSION.isBefore("11")) {
if (!isJDKCompiler || JavaVersion.current().isBefore("11")) {
assertFullyCovered(line);
} else {
assertEmpty(line);
Expand All @@ -40,7 +41,7 @@ public void assertReturnInBodyClose(final Line line) {
if (isJDKCompiler) {
// https://bugs.openjdk.java.net/browse/JDK-8134759
// javac 7 and 8 up to 8u92 are affected
if (JAVA_VERSION.isBefore("1.8.0_92")) {
if (JavaVersion.current().isBefore("1.8.0_92")) {
assertFullyCovered(line);
} else {
assertEmpty(line);
Expand All @@ -61,9 +62,9 @@ public void assertHandwritten(final Line line) {
public void assertEmptyClose(final Line line) {
if (!isJDKCompiler) {
assertPartlyCovered(line, 7, 1);
} else if (JAVA_VERSION.isBefore("8")) {
} else if (JavaVersion.current().isBefore("8")) {
assertPartlyCovered(line, 6, 2);
} else if (JAVA_VERSION.isBefore("9")) {
} else if (JavaVersion.current().isBefore("9")) {
assertPartlyCovered(line, 2, 2);
} else {
assertFullyCovered(line);
Expand All @@ -74,9 +75,9 @@ public void assertThrowInBodyClose(final Line line) {
// not filtered
if (!isJDKCompiler) {
assertNotCovered(line, 6, 0);
} else if (JAVA_VERSION.isBefore("9")) {
} else if (JavaVersion.current().isBefore("9")) {
assertNotCovered(line, 4, 0);
} else if (JAVA_VERSION.isBefore("11")) {
} else if (JavaVersion.current().isBefore("11")) {
assertNotCovered(line);
} else {
assertEmpty(line);
Expand Down
7 changes: 4 additions & 3 deletions ...est.validation.java8/src/org/jacoco/core/test/validation/java8/BadCycleInterfaceTest.java
View file
Expand Up @@ -12,6 +12,7 @@
*******************************************************************************/
package org.jacoco.core.test.validation.java8;

import org.jacoco.core.test.validation.JavaVersion;
import org.jacoco.core.test.validation.Source.Line;
import org.jacoco.core.test.validation.ValidationTestBase;
import org.jacoco.core.test.validation.java8.targets.BadCycleInterfaceTarget;
Expand All @@ -28,7 +29,7 @@ public BadCycleInterfaceTest() throws Exception {

@Test
public void method_execution_sequence() throws Exception {
if (JAVA_VERSION.isBefore("1.8.0_152")) {
if (JavaVersion.current().isBefore("1.8.0_152")) {
assertLogEvents("baseclinit", "childdefaultmethod", "childclinit",
"childstaticmethod");
} else {
Expand All @@ -37,7 +38,7 @@ public void method_execution_sequence() throws Exception {
}

public void assertBaseClInit(final Line line) {
if (JAVA_VERSION.isBefore("1.8.0_152")) {
if (JavaVersion.current().isBefore("1.8.0_152")) {
// Incorrect interpetation of JVMS 5.5 in JDK 8 causes a default
// method to be called before the static initializer of an interface
// (see JDK-8098557 and JDK-8164302):
Expand All @@ -51,7 +52,7 @@ public void assertBaseClInit(final Line line) {
}

public void assertChildDefault(final Line line) throws Exception {
if (JAVA_VERSION.isBefore("1.8.0_152")) {
if (JavaVersion.current().isBefore("1.8.0_152")) {
// Incorrect interpetation of JVMS 5.5 in JDK 8 causes a default
// method to be called before the static initializer of an interface
// (see JDK-8098557 and JDK-8164302):
Expand Down
7 changes: 7 additions & 0 deletions org.jacoco.core.test/src/org/jacoco/core/test/validation/JavaVersion.java
View file
Expand Up @@ -64,4 +64,11 @@ public boolean isBefore(final String version) {
&& this.update < other.update);
}

/**
* @return version of the current JVM
*/
public static JavaVersion current() {
return new JavaVersion(System.getProperty("java.version"));
}

}
3 changes: 0 additions & 3 deletions org.jacoco.core.test/src/org/jacoco/core/test/validation/ValidationTestBase.java
View file
Expand Up @@ -41,9 +41,6 @@ public abstract class ValidationTestBase {

protected static final boolean isJDKCompiler = Compiler.DETECT.isJDK();

protected static final JavaVersion JAVA_VERSION = new JavaVersion(
System.getProperty("java.version"));

private static final String[] STATUS_NAME = new String[4];

{
Expand Down
2 changes: 2 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
View file
Expand Up @@ -33,6 +33,8 @@ <h3>New Features</h3>
<li>Branch added by the Kotlin compiler version 1.6.0 and above for "unsafe" cast
operator is filtered out during generation of report
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1266">#1266</a>).</li>
<li>Improved support for multiple JaCoCo runtimes in the same VM
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1057">#1057</a>).</li>
</ul>

<h3>Fixed bugs</h3>
Expand Down

0 comments on commit aa16a7c

Please sign in to comment.