Skip to content

Commit

Permalink
Ensure single reads of coverage data and single lock.
Browse files Browse the repository at this point in the history
This may address some NPE and hangs when requesting coverage
output in a multi-threaded system.

See #5882.
  • Loading branch information
headius committed Sep 21, 2019
1 parent 5c5d8d3 commit 5c8fdef
Showing 1 changed file with 21 additions and 11 deletions.
32 changes: 21 additions & 11 deletions core/src/main/java/org/jruby/ext/coverage/CoverageData.java
Expand Up @@ -83,7 +83,7 @@ public synchronized Map<String, int[]> resetCoverage(Ruby runtime) {
return coverage;
}

private boolean hasCodeBeenPartiallyCovered(int[] lines) {
private static boolean hasCodeBeenPartiallyCovered(int[] lines) {
for (int i = 0; i < lines.length; i++) {
if (lines[i] > 0) return true;
}
Expand All @@ -94,14 +94,14 @@ private boolean hasCodeBeenPartiallyCovered(int[] lines) {
public synchronized Map<String, int[]> prepareCoverage(String filename, int[] lines) {
assert lines != null;

Map<String, int[]> coverage = this.coverage;

if (filename == null) {
// null filename from certain evals, Ruby.executeScript, etc (jruby/jruby#5111)
// we opt to ignore scripts with no filename, since coverage means nothing
return coverage;
}

Map<String, int[]> coverage = this.coverage;

if (coverage != null) {
coverage.put(filename, lines);
}
Expand All @@ -113,14 +113,24 @@ public synchronized Map<String, int[]> prepareCoverage(String filename, int[] li

private final EventHook COVERAGE_HOOK = new EventHook() {
@Override
public synchronized void eventHandler(ThreadContext context, String eventName, String file, int line, String name, IRubyObject type) {
if (coverage == null || line <= 0) return; // Should not be needed but I predict serialization of IR might hit this.

int[] lines = coverage.get(file);
if (lines == null) return; // no coverage lines for this record. bail out (should never happen)
if (lines.length == 0) return; // coverage is dead for this record. result() has been called once
// and we marked it as such as an empty list.
lines[line - 1] += 1; // increment usage count by one.
public void eventHandler(ThreadContext context, String eventName, String file, int line, String name, IRubyObject type) {
synchronized (CoverageData.this) {
Map<String, int[]> coverage = CoverageData.this.coverage;

// Should not be needed but I predict serialization of IR might hit this.
if (coverage == null || line <= 0) return;

int[] lines = coverage.get(file);

// no coverage lines for this record. bail out (should never happen)
if (lines == null) return;

// coverage is dead for this record. result() has been called once and we marked it as such as an empty list.
if (lines.length == 0) return;

// increment usage count by one.
lines[line - 1] += 1;
}
}

@Override
Expand Down

0 comments on commit 5c8fdef

Please sign in to comment.