From 2cdab8d75c88e46bf3794a95fe0c72f654a00eb9 Mon Sep 17 00:00:00 2001 From: tajila Date: Wed, 5 Feb 2025 09:27:15 -0500 Subject: [PATCH] Prevent doube stop of JFR recording There is an issue where jfr recording can be stopped twice. When this occurs buffers that are already free'd are freed a second time. Signed-off-by: tajila --- .../tools/attach/target/DiagnosticUtils.java | 4 +- runtime/jcl/common/com_ibm_oti_vm_VM.cpp | 20 +++-- test/functional/cmdLineTests/jfr/jfr.xml | 12 ++- .../jfr/src/org/openj9/test/VMAPITest.java | 7 +- .../jfr/src/org/openj9/test/VMAPITest2.java | 77 +++++++++++++++++++ 5 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest2.java diff --git a/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/DiagnosticUtils.java b/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/DiagnosticUtils.java index 1cda70ec78b..78bd216d3b5 100644 --- a/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/DiagnosticUtils.java +++ b/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/DiagnosticUtils.java @@ -484,7 +484,9 @@ private static DiagnosticProperties doJFR(String diagnosticCommand) { Timer timer = new Timer(); TimerTask jfrDumpTask = new TimerTask() { public void run() { - VM.stopJFR(); + if (VM.isJFRRecordingStarted()) { + VM.stopJFR(); + } } }; timer.schedule(jfrDumpTask, duration); diff --git a/runtime/jcl/common/com_ibm_oti_vm_VM.cpp b/runtime/jcl/common/com_ibm_oti_vm_VM.cpp index 8225d9f78c6..dc964c16454 100644 --- a/runtime/jcl/common/com_ibm_oti_vm_VM.cpp +++ b/runtime/jcl/common/com_ibm_oti_vm_VM.cpp @@ -255,8 +255,10 @@ Java_com_ibm_oti_vm_VM_startJFR(JNIEnv *env, jclass unused) J9JavaVM *vm = currentThread->javaVM; J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions; - /* this is to initalize JFR late after VM startup */ - rc = vmFuncs->initializeJFR(vm, TRUE); + if (!vmFuncs->isJFRRecordingStarted(vm)) { + /* this is to initalize JFR late after VM startup */ + rc = vmFuncs->initializeJFR(vm, TRUE); + } return rc; } @@ -268,12 +270,14 @@ Java_com_ibm_oti_vm_VM_stopJFR(JNIEnv *env, jclass unused) J9JavaVM *vm = currentThread->javaVM; J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions; - vmFuncs->internalEnterVMFromJNI(currentThread); - vmFuncs->acquireExclusiveVMAccess(currentThread); - vmFuncs->jfrDump(currentThread, TRUE); - vmFuncs->releaseExclusiveVMAccess(currentThread); - vmFuncs->tearDownJFR(vm); - vmFuncs->internalExitVMToJNI(currentThread); + if (vmFuncs->isJFRRecordingStarted(vm)) { + vmFuncs->internalEnterVMFromJNI(currentThread); + vmFuncs->acquireExclusiveVMAccess(currentThread); + vmFuncs->jfrDump(currentThread, TRUE); + vmFuncs->releaseExclusiveVMAccess(currentThread); + vmFuncs->tearDownJFR(vm); + vmFuncs->internalExitVMToJNI(currentThread); + } } void JNICALL diff --git a/test/functional/cmdLineTests/jfr/jfr.xml b/test/functional/cmdLineTests/jfr/jfr.xml index b6a11fafe0a..8e179bb11ba 100644 --- a/test/functional/cmdLineTests/jfr/jfr.xml +++ b/test/functional/cmdLineTests/jfr/jfr.xml @@ -24,7 +24,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex - + $EXE$ -XX:StartFlightRecording --add-exports java.base/com.ibm.oti.vm=ALL-UNNAMED -cp $RESJAR$ org.openj9.test.TriggerExecutionSample @@ -45,6 +45,16 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex All runs complete. Failed + + $EXE$ --add-exports java.base/com.ibm.oti.vm=ALL-UNNAMED -cp $RESJAR$ org.openj9.test.VMAPITest 1 + All runs complete. + Failed + + + $EXE$ --add-exports java.base/com.ibm.oti.vm=ALL-UNNAMED -cp $RESJAR$ org.openj9.test.VMAPITest2 + All runs complete. + Failed + $EXE$ --add-exports java.base/com.ibm.oti.vm=ALL-UNNAMED -cp $RESJAR$ org.openj9.test.JFRCMDLineTest All runs complete diff --git a/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest.java b/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest.java index fdf29174aa0..ee34e8a1bf2 100644 --- a/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest.java +++ b/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest.java @@ -26,6 +26,11 @@ public class VMAPITest { public static void main(String[] args) throws Throwable { final WorkLoad workLoad = new WorkLoad(200, 20000, 200); + int sleepDuration = 1000; + + if (args.length > 1) { + sleepDuration = Integer.parseInt(args[0]); + } if (VM.isJFRRecordingStarted()) { System.out.println("Failed should not be recording."); @@ -55,7 +60,7 @@ public static void main(String[] args) throws Throwable { return; } - Thread.sleep(1000); + Thread.sleep(sleepDuration); VM.jfrDump(); VM.stopJFR(); diff --git a/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest2.java b/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest2.java new file mode 100644 index 00000000000..e1cb2e7180b --- /dev/null +++ b/test/functional/cmdLineTests/jfr/src/org/openj9/test/VMAPITest2.java @@ -0,0 +1,77 @@ +/* + * Copyright IBM Corp. and others 2024 + * + * This program and the accompanying materials are made available under + * the terms of the Eclipse Public License 2.0 which accompanies this + * distribution and is available at https://www.eclipse.org/legal/epl-2.0/ + * or the Apache License, Version 2.0 which accompanies this distribution and + * is available at https://www.apache.org/licenses/LICENSE-2.0. + * + * This Source Code may also be made available under the following + * Secondary Licenses when the conditions for such availability set + * forth in the Eclipse Public License, v. 2.0 are satisfied: GNU + * General Public License, version 2 with the GNU Classpath + * Exception [1] and GNU General Public License, version 2 with the + * OpenJDK Assembly Exception [2]. + * + * [1] https://www.gnu.org/software/classpath/license.html + * [2] https://openjdk.org/legal/assembly-exception.html + * + * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 OR GPL-2.0-only WITH OpenJDK-assembly-exception-1.0 + */ +package org.openj9.test; + +import com.ibm.oti.vm.VM; + +public class VMAPITest2 { + public static void main(String[] args) throws Throwable { + final WorkLoad workLoad = new WorkLoad(200, 20000, 200); + + Thread app = new Thread(() -> { + workLoad.runWork(); + }); + app.start(); + + try { + Thread.sleep(3000); + } catch(Throwable t) { + t.printStackTrace(); + } + + if (VM.isJFRRecordingStarted()) { + System.out.println("Failed should not be recording 1."); + System.exit(0); + } + + /* attempt a stop recording before it has started */ + VM.stopJFR(); + + if (VM.isJFRRecordingStarted()) { + System.out.println("Failed should not be recording 2."); + System.exit(0); + } + + VM.startJFR(); + if (!VM.isJFRRecordingStarted()) { + System.out.println("Failed should be recording 3."); + System.exit(0); + } + + + /* attempt another start recording after one has already started */ + VM.startJFR(); + if (!VM.isJFRRecordingStarted()) { + System.out.println("Failed should be recording 4."); + System.exit(0); + } + + VM.stopJFR(); + if (VM.isJFRRecordingStarted()) { + System.out.println("Failed should not be recording 5."); + System.exit(0); + } + + /* attempt a stop recording before it has started */ + VM.stopJFR(); + } +}