[GR-53385] Fix: DeltaBlue benchmark buffer leak.

PullRequest: graal/17691
oracle · May 18, 2024 · 6eea094 · 6eea094
2 parents 978cadd + 06e8304
commit 6eea094
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 8 deletions.
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrRecorderThread.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrRecorderThread.java
@@ -24,7 +24,8 @@
  */
 package com.oracle.svm.core.jfr;
 
-import jdk.graal.compiler.core.common.SuppressFBWarnings;
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.word.UnsignedWord;
@@ -36,6 +37,8 @@
 import com.oracle.svm.core.sampler.SamplerBuffersAccess;
 import com.oracle.svm.core.util.VMError;
 
+import jdk.graal.compiler.core.common.SuppressFBWarnings;
+
 /**
  * A daemon thread that is created during JFR startup and torn down by
  * {@link SubstrateJVM#destroyJFR}.
@@ -54,6 +57,7 @@ public class JfrRecorderThread extends Thread {
  private final JfrUnlockedChunkWriter unlockedChunkWriter;
 
  private final VMSemaphore semaphore;
+ private final ReentrantLock lock;
  /* A volatile boolean field would not be enough to ensure synchronization. */
  private final UninterruptibleUtils.AtomicBoolean atomicNotify;
 
@@ -66,6 +70,7 @@ public JfrRecorderThread(JfrGlobalMemory globalMemory, JfrUnlockedChunkWriter un
  this.globalMemory = globalMemory;
  this.unlockedChunkWriter = unlockedChunkWriter;
  this.semaphore = new VMSemaphore("jfrRecorder");
+ this.lock = new ReentrantLock();
  this.atomicNotify = new UninterruptibleUtils.AtomicBoolean(false);
  setDaemon(true);
  }
@@ -75,7 +80,12 @@ public void run() {
  try {
  while (!stopped) {
  if (await()) {
- run0();
+ lock.lock();
+ try {
+ work();
+ } finally {
+ lock.unlock();
+ }
  }
  }
  } catch (Throwable e) {
@@ -88,7 +98,7 @@ private boolean await() {
  return atomicNotify.compareAndSet(true, false);
  }
 
- private void run0() {
+ private void work() {
  SamplerBuffersAccess.processFullBuffers(true);
  JfrChunkWriter chunkWriter = unlockedChunkWriter.lock();
  try {
@@ -100,6 +110,16 @@ private void run0() {
  }
  }
 
+ void endRecording() {
+ lock.lock();
+ try {
+ SubstrateJVM.JfrEndRecordingOperation vmOp = new SubstrateJVM.JfrEndRecordingOperation();
+ vmOp.enqueue();
+ } finally {
+ lock.unlock();
+ }
+ }
+
  @SuppressFBWarnings(value = "NN_NAKED_NOTIFY", justification = "state change is in native buffer")
  private void persistBuffers(JfrChunkWriter chunkWriter) {
  JfrBufferList buffers = globalMemory.getBuffers();

diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java
@@ -26,7 +26,6 @@
 
 import java.util.List;
 
-import com.oracle.svm.core.sampler.SamplerStatistics;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.Platform;
@@ -45,6 +44,8 @@
 import com.oracle.svm.core.jfr.throttling.JfrEventThrottling;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.sampler.SamplerBufferPool;
+import com.oracle.svm.core.sampler.SamplerBuffersAccess;
+import com.oracle.svm.core.sampler.SamplerStatistics;
 import com.oracle.svm.core.sampler.SubstrateSigprofHandler;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.JavaVMOperation;
@@ -349,8 +350,7 @@ public void endRecording() {
  return;
  }
 
- JfrEndRecordingOperation vmOp = new JfrEndRecordingOperation();
- vmOp.enqueue();
+ recorderThread.endRecording();
  }
 
  /**
@@ -748,7 +748,7 @@ protected void operate() {
  }
  }
 
- private static class JfrEndRecordingOperation extends JavaVMOperation {
+ static class JfrEndRecordingOperation extends JavaVMOperation {
  JfrEndRecordingOperation() {
  super(VMOperationInfos.get(JfrEndRecordingOperation.class, "JFR end recording", SystemEffect.SAFEPOINT));
  }
@@ -760,6 +760,10 @@ private static class JfrEndRecordingOperation extends JavaVMOperation {
  */
  @Override
  protected void operate() {
+ if (!SubstrateJVM.get().recording) {
+ return;
+ }
+
  SubstrateJVM.get().recording = false;
  JfrExecutionSampler.singleton().update();
 
@@ -772,6 +776,9 @@ protected void operate() {
  JfrThreadLocal.stopRecording(isolateThread, false);
  }
 
+ /* Process any remaining full buffers (if there are any). */
+ SamplerBuffersAccess.processFullBuffers(false);
+
  /*
  * If JFR recording is restarted later on, then it needs to start with a clean state.
  * Therefore, we clear all data that is still pending.

diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferPool.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferPool.java
@@ -59,7 +59,7 @@ public SamplerBufferPool() {
 
  public void teardown() {
  clear(availableBuffers);
- clear(fullBuffers);
+ /* There should not be any unprocessed buffers. */
  assert bufferCount == 0;
  }