Merge pull request #21070 from JasonFengJ9/jfrjcmd

JFR can invoke VM.setJFRRecordingFileName() before VM.startJFR()
eclipse-openj9 · Feb 6, 2025 · 516d0f5 · 516d0f5
2 parents f6f1da0 + c28c696
commit 516d0f5
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 18 deletions.
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
@@ -448,35 +448,33 @@ private static String parseStringParameter(String paramName, String[] parameters
 
 	private static DiagnosticProperties doJFR(String diagnosticCommand) {
 		DiagnosticProperties result = null;
-
 		// split the command and arguments
 		String[] parts = diagnosticCommand.split(DIAGNOSTICS_OPTION_SEPARATOR);
 		IPC.logMessage("doJFR: ", diagnosticCommand);
-
 		// ensure there's at least one part for the command
 		if (parts.length == 0) {
 			return DiagnosticProperties.makeErrorProperties("Error: No JFR command specified");
 		}
-
 		String command = parts[0].trim();
 		String[] parameters = Arrays.copyOfRange(parts, 1, parts.length);
-
 		String fileName = parseStringParameter("filename", parameters, null);
 		IPC.logMessage("doJFR: filename = ", fileName);
-		boolean setFileName = (fileName != null) && !fileName.isEmpty();
-		if (setFileName) {
-			if (!VM.setJFRRecordingFileName(fileName)) {
-				return DiagnosticProperties.makeErrorProperties("setJFRRecordingFileName failed");
-			} else {
-				jfrRecordingFileName = fileName;
-			}
-		}
 
 		try {
 			if (command.equalsIgnoreCase(DIAGNOSTICS_JFR_START)) {
 				if (VM.isJFRRecordingStarted()) {
 					result = DiagnosticProperties.makeErrorProperties("One JFR recording is in progress [" + jfrRecordingFileName + "], only one recording is allowed at a time.");
 				} else {
+					// only JFR.start command is allowed to change the recording filename
+					boolean setFileName = (fileName != null) && !fileName.isEmpty();
+					if (setFileName) {
+						// the recording filename should be set before VM.startJFR() which invokes JFRWriter:openJFRFile()
+						if (!VM.setJFRRecordingFileName(fileName)) {
+							return DiagnosticProperties.makeErrorProperties("setJFRRecordingFileName() failed");
+						} else {
+							jfrRecordingFileName = fileName;
+						}
+					}
 					VM.startJFR();
 					long duration = parseTimeParameter("duration", parameters);
 					IPC.logMessage("doJFR: duration = " + duration);

diff --git a/runtime/vm/jfr.cpp b/runtime/vm/jfr.cpp
@@ -1127,12 +1127,15 @@ jfrSamplingThreadProc(void *entryArg)
 jboolean
 setJFRRecordingFileName(J9JavaVM *vm, char *fileName)
 {
-	UDATA defaultFileNameLen = strlen(DEFAULT_JFR_FILE_NAME);
-	if ((defaultFileNameLen != strlen(vm->jfrState.jfrFileName))
-	|| (0 != strncmp(DEFAULT_JFR_FILE_NAME, vm->jfrState.jfrFileName, defaultFileNameLen))
-	) {
-		PORT_ACCESS_FROM_JAVAVM(vm);
-		j9mem_free_memory(vm->jfrState.jfrFileName);
+	if (NULL != vm->jfrState.jfrFileName) {
+		UDATA defaultFileNameLen = strlen(DEFAULT_JFR_FILE_NAME);
+		const char *jfrFileName = vm->jfrState.jfrFileName;
+		if ((defaultFileNameLen != strlen(jfrFileName))
+				|| (0 != strncmp(DEFAULT_JFR_FILE_NAME, jfrFileName, defaultFileNameLen))
+		) {
+			PORT_ACCESS_FROM_JAVAVM(vm);
+			j9mem_free_memory(vm->jfrState.jfrFileName);
+		}
 	}
 	vm->jfrState.jfrFileName = fileName;
 	VM_JFRWriter::closeJFRFile(vm);