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

Improve support for multiple JaCoCo runtimes in the same VM #1057

Merged
merged 10 commits into from Apr 4, 2022
Merged

Improve support for multiple JaCoCo runtimes in the same VM #1057

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c350ef6
Retry exec file locking when file is locked from same JVM
marchof Jul 5, 2021
c74b012
OverlappingFileLockException only thrown since Java 1.6
marchof Jul 5, 2021
7a11b6b
Consolidate place to check current JDK version
marchof Jul 5, 2021
bc94706
Update change log
marchof Jul 5, 2021
25a3cbb
Review Comment: Use try/catch pattern for exception assertion
marchof Jul 6, 2021
6b4fa1b
Fix typo
marchof Jul 8, 2021
dd60a52
Review: Fix type
marchof Jul 8, 2021
d07386c
Review: Fix typo
marchof Jul 8, 2021
de55b08
Review comment: Adjust test method naming to actual test case
marchof Jul 8, 2021
b5283e9
Merge branch 'master' into retry-execfile-lock
Godin Apr 4, 2022
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
4 changes: 4 additions & 0 deletions org.jacoco.agent.rt.test/pom.xml
View file
Open in desktop
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
56 changes: 53 additions & 3 deletions org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/output/FileOutputTest.java
View file
Open in desktop
Expand Up @@ -16,10 +16,15 @@
import static org.junit.Assert.assertTrue;

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 +38,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 +52,7 @@ public void testCreateDestFileOnStartup() throws Exception {
}

@Test
public void testWriteData() throws Exception {
public void shutdown_should_write_execdata() throws Exception {
marchof marked this conversation as resolved.
Show resolved Hide resolved
File destFile = folder.newFile("jacoco.exec");
AgentOptions options = new AgentOptions();
options.setDestfile(destFile.getAbsolutePath());
Expand All @@ -63,7 +68,8 @@ public void testWriteData() throws Exception {
}

@Test(expected = IOException.class)
public void testInvalidDestFile() throws Exception {
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();
Expand All @@ -72,4 +78,48 @@ public void testInvalidDestFile() throws Exception {
controller.startup(options, new RuntimeData());
}

@Test(expected = OverlappingFileLockException.class)
marchof marked this conversation as resolved.
Show resolved Hide resolved
public void startup_should_throws_OverlappingFileLockException_when_execfile_is_permanently_locked()
marchof marked this conversation as resolved.
Show resolved Hide resolved
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);
try {
out.getChannel().lock();
FileOutput controller = new FileOutput();
controller.startup(options, new RuntimeData());
} finally {
out.close();
}
}

@Test(expected = InterruptedIOException.class)
public void startup_should_throws_InterruptedIOException_when_execfile_is_locked_and_thread_is_interrupted()
marchof marked this conversation as resolved.
Show resolved Hide resolved
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);
try {
out.getChannel().lock();
FileOutput controller = new FileOutput();
Thread.currentThread().interrupt();
controller.startup(options, new RuntimeData());
} finally {
out.close();
}
}
}
31 changes: 29 additions & 2 deletions org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/output/FileOutput.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Expand Up @@ -64,4 +64,11 @@ public boolean isBefore(final String version) {
&& this.update < other.update);
}

/**
* @return Version of the current JVM
marchof marked this conversation as resolved.
Show resolved Hide resolved
*/
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
Open in desktop
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
Open in desktop
Expand Up @@ -27,6 +27,8 @@ <h3>New Features</h3>
<li>Part of bytecode generated by the Java compilers for <code>assert</code>
statement is filtered out during generation of report
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1196">#1196</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