Skip to content

Commit

Permalink
Refactor PauseDetectorWrapper retrieval #1300
Browse files Browse the repository at this point in the history
Previously, PauseDetectorWrapper was initialized once and a later loop attempted to retrieve the PauseDetector assuming that the previous code would had initialized the PauseDetector.

Due to concurrency it is possible that the PauseDetector instance is being released while another thread wants to obtain the PauseDetector. This gap could lead to infinite loops as the instance is never allocated. The refactored code include now the initialization so concurrent initialization/release are now properly guarded.
  • Loading branch information
mp911de committed Jun 8, 2020
1 parent a076b77 commit 4db44dc
Showing 1 changed file with 13 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,25 @@ public void recordCommandLatency(SocketAddress local, SocketAddress remote, Prot
return;
}

if (PAUSE_DETECTOR_UPDATER.get(this) == null) {
if (PAUSE_DETECTOR_UPDATER.compareAndSet(this, null, GLOBAL_PAUSE_DETECTOR)) {
PAUSE_DETECTOR_UPDATER.get(this).retain();
PauseDetector pauseDetector;

do {
if (PAUSE_DETECTOR_UPDATER.get(this) == null) {
if (PAUSE_DETECTOR_UPDATER.compareAndSet(this, null, GLOBAL_PAUSE_DETECTOR)) {
PAUSE_DETECTOR_UPDATER.get(this).retain();
}
}
}
PauseDetector pauseDetector = ((DefaultPauseDetectorWrapper) PAUSE_DETECTOR_UPDATER.get(this)).getPauseDetector();
pauseDetector = ((DefaultPauseDetectorWrapper) PAUSE_DETECTOR_UPDATER.get(this)).getPauseDetector();
} while (pauseDetector == null);

PauseDetector pauseDetectorToUse = pauseDetector;
Latencies latencies = latencyMetricsRef.get().computeIfAbsent(createId(local, remote, commandType), id -> {

if (options.resetLatenciesAfterEvent()) {
return new Latencies(pauseDetector);
return new Latencies(pauseDetectorToUse);
}

return new CummulativeLatencies(pauseDetector);
return new CummulativeLatencies(pauseDetectorToUse);
});

latencies.firstResponse.recordLatency(rangify(firstResponseLatency));
Expand Down Expand Up @@ -352,18 +357,10 @@ static class DefaultPauseDetectorWrapper implements PauseDetectorWrapper {

/**
* Obtain the current {@link PauseDetector}. Requires a call to {@link #retain()} first.
*
*
* @return
*/
public PauseDetector getPauseDetector() {

for (;;) {

if (pauseDetector != null) {
break;
}
}

return pauseDetector;
}

Expand Down

0 comments on commit 4db44dc

Please sign in to comment.