Merge pull request #21356 from tajila/jfrbackport

(0.51) Fix owner for JFR monitor enter

Author: babsingh

runtime/vm/JFRChunkWriter.hpp

@@ -596,13 +596,13 @@ class VM_JFRChunkWriter {
 		VM_BufferWriter *_bufferWriter = (VM_BufferWriter *)userData;
 
 		/* reserve size field */
-		U_8 *dataStart = _bufferWriter->getAndIncCursor(sizeof(U_32));
+		U_8 *dataStart = reserveEventSize(_bufferWriter);
 
 		/* write event type */
 		_bufferWriter->writeLEB128(MonitorEnterID);
 
-		/* write start time - this is when the sleep started not when it ended so we
-		 * need to subtract the duration since the event is emitted when the sleep ends.
+		/* write start time - this is when the monitor enter started not when it ended so we
+		 * need to subtract the duration since the event is emitted when the monitor enter ends.
 		 */
 		_bufferWriter->writeLEB128(entry->ticks - entry->duration);
 

runtime/vm/JFRConstantPoolTypes.cpp

@@ -770,12 +770,25 @@ VM_JFRConstantPoolTypes::addThreadEntry(J9VMThread *vmThread)
 	U_32 index = U_32_MAX;
 	ThreadEntry *entry = NULL;
 	ThreadEntry entryBuffer = {0};
+	omrthread_t osThread = NULL;
+	j9object_t threadObject = NULL;
+
+	if (NULL == vmThread) {
+		index = 0;
+		goto done;
+	}
 
 	entry = &entryBuffer;
 	entry->vmThread = vmThread;
 	_buildResult = OK;
-	omrthread_t osThread = vmThread->osThread;
-	j9object_t threadObject = vmThread->threadObject;
+	osThread = vmThread->osThread;
+	threadObject = vmThread->threadObject;
+
+	if ((NULL == osThread) || (NULL == threadObject)) {
+		/* this can happen if a thread dies during a monitor enter */
+		index = 0;
+		goto done;
+	}
 
 	entry = (ThreadEntry *) hashTableFind(_threadTable, entry);
 	if (NULL != entry) {
@@ -1102,6 +1115,9 @@ VM_JFRConstantPoolTypes::addMonitorEnterEntry(J9JFRMonitorEntered *monitorEnterD
 	entry->threadIndex = addThreadEntry(monitorEnterData->vmThread);
 	if (isResultNotOKay()) goto done;
 
+	entry->previousOwnerThread = addThreadEntry(monitorEnterData->previousOwner);
+	if (isResultNotOKay()) goto done;
+
 	entry->eventThreadIndex = addThreadEntry(monitorEnterData->vmThread);
 	if (isResultNotOKay()) goto done;
 

runtime/vm/ObjectMonitor.cpp

@@ -168,6 +168,7 @@ objectMonitorEnterBlocking(J9VMThread *currentThread)
 		object = NULL; // for safety, since object may be moved by the GC at various points after this
 		/* Ensure object monitor isn't deflated while we block */
 		omrthread_monitor_t monitor = objectMonitor->monitor;
+		J9VMThread *previousOwner = getVMThreadFromOMRThread(vm, ((J9ThreadMonitor *)monitor)->owner);
 		VM_AtomicSupport::add(&monitor->pinCount, 1);
 		/* Initialize our wait time to 1ms. Increase it as we have to wait more and more
 		 * using the sequence 1, 4, 16, 64 and then 64 thereafter.
@@ -291,9 +292,8 @@ objectMonitorEnterBlocking(J9VMThread *currentThread)
 		((J9ThreadMonitor*)monitor)->flags &= ~(UDATA)J9THREAD_MONITOR_SUPPRESS_CONTENDED_EXIT;
 		VM_AtomicSupport::subtract(&monitor->pinCount, 1);
 		if (J9_EVENT_IS_HOOKED(vm->hookInterface, J9HOOK_VM_MONITOR_CONTENDED_ENTERED)) {
-			J9VMThread *ownerThread = getVMThreadFromOMRThread(vm, ((J9ThreadMonitor *)monitor)->owner);
 			bool frameBuilt = saveBlockingEnterObject(currentThread);
-			ALWAYS_TRIGGER_J9HOOK_VM_MONITOR_CONTENDED_ENTERED(vm->hookInterface, currentThread, monitor, startTicks, ramClass, ownerThread);
+			ALWAYS_TRIGGER_J9HOOK_VM_MONITOR_CONTENDED_ENTERED(vm->hookInterface, currentThread, monitor, startTicks, ramClass, previousOwner);
 			restoreBlockingEnterObject(currentThread, frameBuilt);
 		}
 	}

runtime/vm/jfr.cpp

@@ -696,6 +696,7 @@ jfrVMMonitorEntered(J9HookInterface **hook, UDATA eventNum, void *eventData, voi
 		jfrEvent->duration = j9time_nano_time() - event->startTicks;
 		jfrEvent->monitorClass = event->monitorClass;
 		jfrEvent->monitorAddress = (UDATA)event->monitor;
+		jfrEvent->previousOwner = event->previousOwner;
 	}
 }
 

test/functional/cmdLineTests/jfr/jfrevents.xml

@@ -64,4 +64,15 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex
 		<output type="success" caseSensitive="yes" regex="no">stackTrace</output>
 		<output type="failure" caseSensitive="yes" regex="no">jfr print: could not read recording</output>
 	</test>
+	<test id="test jfr monitor enter - approx 30seconds">
+		<command>$JFR_EXE$ print --xml --events "JavaMonitorEnter" --stack-depth 1 defaultJ9recording.jfr</command>
+		<output type="required" caseSensitive="yes" regex="no">http://www.w3.org/2001/XMLSchema-instance</output>
+		<output type="required" caseSensitive="yes" regex="no">jdk.JavaMonitorEnter</output>
+		<output type="required" caseSensitive="yes" regex="no">eventThread</output>
+		<output type="required" caseSensitive="yes" regex="no">osName</output>
+		<output type="required" caseSensitive="yes" regex="no">osThreadId</output>
+		<output type="required" caseSensitive="yes" regex="no">javaName</output>
+		<output type="success" caseSensitive="yes" regex="no">stackTrace</output>
+		<output type="failure" caseSensitive="yes" regex="no">jfr print: could not read recording</output>
+	</test>
 </suite>

test/functional/cmdLineTests/jfr/src/org/openj9/test/WorkLoad.java

@@ -35,6 +35,10 @@ public class WorkLoad {
 	public static double average;
 	public static double stdDev;
 
+	private static long globalCounter = 0;
+	static interface GlobalLoack {};
+	private static Object globalLock = new GlobalLoack(){};
+
 	public WorkLoad(int numberOfThreads, int sizeOfNumberList, int repeats) {
 		this.numberOfThreads = numberOfThreads;
 		this.sizeOfNumberList = sizeOfNumberList;
@@ -93,10 +97,22 @@ private void workload() {
 			generateTimedSleep();
 			generateTimedWait();
 			throwThrowables();
+			contendOnLock();
 			burnCPU();
 		}
 	}
 
+	private void contendOnLock() {
+		synchronized (globalLock) {
+			globalCounter++;
+			try {
+				Thread.sleep(1);
+			} catch (InterruptedException e) {
+				e.printStackTrace();
+			}
+		}
+	}
+
 	private void recursiveFucntion(int depth) {
 		if (0 == depth) {
 			return;