Merge pull request #15201 from gacholio/safe

Fix halted thread being allowed to continue execution

runtime/gc_base/IdleGCManager.cpp

@@ -1,6 +1,5 @@
-
 /*******************************************************************************
- * Copyright (c) 1991, 2020 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -33,6 +32,7 @@
 #include "GCExtensions.hpp"
 #include "OMRVMInterface.hpp"
 #include "Heap.hpp"
+#include "VMAccess.hpp"
 
 MM_IdleGCManager *
 MM_IdleGCManager::newInstance(MM_EnvironmentBase* env)
@@ -81,7 +81,9 @@ MM_IdleGCManager::manageFreeHeap(J9VMThread* currentThread)
 	MM_GCExtensions* _extensions = MM_GCExtensions::getExtensions(env);
 
 	_javaVM->internalVMFunctions->internalAcquireVMAccess(currentThread);
+	VM_VMAccess::setPublicFlags(currentThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 	_extensions->heap->systemGarbageCollect(env, J9MMCONSTANT_EXPLICIT_GC_IDLE_GC);
+	VM_VMAccess::clearPublicFlags(currentThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 	_javaVM->internalVMFunctions->internalReleaseVMAccess(currentThread);
 }
 

runtime/gc_base/modronapi.cpp

@@ -1,6 +1,5 @@
-
 /*******************************************************************************
- * Copyright (c) 1991, 2020 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -48,6 +47,7 @@
 #include "MemoryPoolLargeObjects.hpp"
 #include "VMInterface.hpp"
 #include "VMThreadListIterator.hpp"
+#include "VMAccess.hpp"
 
 extern "C" {
 
@@ -86,8 +86,15 @@ j9gc_modron_global_collect_with_overrides(J9VMThread *vmThread, U_32 gcCode)
 		}
 	}
 
+	VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 	MM_GCExtensions::getExtensions(env)->heap->systemGarbageCollect(env, gcCode);
-
+	VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
+	if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_ANY)) {
+		vmThread->javaVM->internalVMFunctions->internalReleaseVMAccess(vmThread);
+		vmThread->javaVM->internalVMFunctions->internalAcquireVMAccess(vmThread);
+	}
+
 	return 0;
 }
 
@@ -96,7 +103,14 @@ j9gc_modron_local_collect(J9VMThread *vmThread)
 {
 	MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread);
 
+	VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 	((MM_MemorySpace *)vmThread->omrVMThread->memorySpace)->localGarbageCollect(env, J9MMCONSTANT_IMPLICIT_GC_DEFAULT);
+	VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
+	if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_ANY)) {
+		vmThread->javaVM->internalVMFunctions->internalReleaseVMAccess(vmThread);
+		vmThread->javaVM->internalVMFunctions->internalAcquireVMAccess(vmThread);
+	}
 
 	return 0;
 }

runtime/gc_glue_java/ConcurrentMarkingDelegate.cpp

@@ -31,6 +31,7 @@
 #include "HeapRegionIteratorStandard.hpp"
 #include "ReferenceObjectList.hpp"
 #include "StackSlotValidator.hpp"
+#include "VMAccess.hpp"
 #include "VMInterface.hpp"
 #include "VMThreadListIterator.hpp"
 
@@ -450,4 +451,19 @@ MM_ConcurrentMarkingDelegate::concurrentClassMark(MM_EnvironmentBase *env, bool
 }
 #endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
 
+void
+MM_ConcurrentMarkingDelegate::acquireExclusiveVMAccessAndSignalThreadsToActivateWriteBarrier(MM_EnvironmentBase *env)
+{
+	J9VMThread *vmThread = (J9VMThread *)env->getLanguageVMThread();
+
+	VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+	_collector->acquireExclusiveVMAccessAndSignalThreadsToActivateWriteBarrier(env);
+	VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
+	if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_ANY)) {
+		vmThread->javaVM->internalVMFunctions->internalReleaseVMAccess(vmThread);
+		vmThread->javaVM->internalVMFunctions->internalAcquireVMAccess(vmThread);
+	}
+}
+
 #endif /* defined(OMR_GC_MODRON_CONCURRENT_MARK) */

runtime/gc_glue_java/ConcurrentMarkingDelegate.hpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 1991, 2021 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -249,6 +249,8 @@ class MM_ConcurrentMarkingDelegate
 
 	void signalThreadsToDeactivateWriteBarrier(MM_EnvironmentBase *env);
 
+	void acquireExclusiveVMAccessAndSignalThreadsToActivateWriteBarrier(MM_EnvironmentBase *env);
+
 	/**
 	 * This method is called during card cleaning for each object associated with an uncleaned, dirty card in the card
 	 * table. No client actions are necessary but this method may be overridden if desired to hook into card cleaning.

runtime/gc_glue_java/EnvironmentDelegate.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2017, 2021 IBM Corp. and others
+ * Copyright (c) 2017, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -234,15 +234,13 @@ MM_EnvironmentDelegate::assumeExclusiveVMAccess(uintptr_t exclusiveCount)
 void
 MM_EnvironmentDelegate::releaseCriticalHeapAccess(uintptr_t *data)
 {
-	VM_VMAccess::setPublicFlags(_vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 	MM_JNICriticalRegion::releaseAccess(_vmThread, data);
 }
 
 void
 MM_EnvironmentDelegate::reacquireCriticalHeapAccess(uintptr_t data)
 {
 	MM_JNICriticalRegion::reacquireAccess(_vmThread, data);
-	VM_VMAccess::clearPublicFlags(_vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
 }
 
 void

runtime/gc_modron_startup/mgcalloc.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 1991, 2021 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -387,6 +387,8 @@ J9AllocateObject(J9VMThread *vmThread, J9Class *clazz, uintptr_t allocateFlags)
 {
 	MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread);
 
+	VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 #if defined(J9VM_GC_THREAD_LOCAL_HEAP)	
 	if (!env->isInlineTLHAllocateEnabled()) {
 		/* For duration of call restore TLH allocate fields;
@@ -454,11 +456,17 @@ J9AllocateObject(J9VMThread *vmThread, J9Class *clazz, uintptr_t allocateFlags)
 				objectPtr, 
 				sizeInBytesRequired);
 		} else {
-			TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
-				vmThread->javaVM->hookInterface, 
-				vmThread,
-				objectPtr, 
-				sizeInBytesRequired);
+			if (J9_EVENT_IS_HOOKED(vmThread->javaVM->hookInterface, J9HOOK_VM_OBJECT_ALLOCATE)) {
+				/* The JIT optimization only uses instrumentable allocate, so clear the NOT_AT_SAFE_POINT
+				 * bit now to allow the hook to run with no restrictions.
+				 */
+				VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+				ALWAYS_TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
+					vmThread->javaVM->hookInterface,
+					vmThread,
+					objectPtr,
+					sizeInBytesRequired);
+			}
 		}
 
 		if( !mixedOAM.getAllocateDescription()->isCompletedFromTlh()) {
@@ -516,6 +524,15 @@ J9AllocateObject(J9VMThread *vmThread, J9Class *clazz, uintptr_t allocateFlags)
 	}
 #endif /* J9VM_GC_THREAD_LOCAL_HEAP */	
 
+	if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_ANY)) {
+		env->saveObjects((omrobjectptr_t)objectPtr);
+		vmThread->javaVM->internalVMFunctions->internalReleaseVMAccess(vmThread);
+		vmThread->javaVM->internalVMFunctions->internalAcquireVMAccess(vmThread);
+		env->restoreObjects((omrobjectptr_t*)&objectPtr);
+	}
+
+	VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 	return objectPtr;
 }
 
@@ -533,6 +550,8 @@ J9AllocateIndexableObject(J9VMThread *vmThread, J9Class *clazz, uint32_t numberO
 	MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread);
 	MM_GCExtensions *extensions = MM_GCExtensions::getExtensions(env);
 
+	VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 	Assert_MM_false(allocateFlags & OMR_GC_ALLOCATE_OBJECT_NO_GC);
 	if (OMR_GC_ALLOCATE_OBJECT_NON_ZERO_TLH == (allocateFlags & OMR_GC_ALLOCATE_OBJECT_NON_ZERO_TLH)) {
 		Assert_MM_true(GC_ObjectModel::SCAN_PRIMITIVE_ARRAY_OBJECT == extensions->objectModel.getScanType(clazz));
@@ -589,11 +608,17 @@ J9AllocateIndexableObject(J9VMThread *vmThread, J9Class *clazz, uint32_t numberO
 				objectPtr, 
 				sizeInBytesRequired);
 		} else {
-			TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
-				vmThread->javaVM->hookInterface, 
-				vmThread, 
-				objectPtr, 
-				sizeInBytesRequired);
+			if (J9_EVENT_IS_HOOKED(vmThread->javaVM->hookInterface, J9HOOK_VM_OBJECT_ALLOCATE)) {
+				/* The JIT optimization only uses instrumentable allocate, so clear the NOT_AT_SAFE_POINT
+				 * bit now to allow the hook to run with no restrictions.
+				 */
+				VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+				TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
+					vmThread->javaVM->hookInterface,
+					vmThread,
+					objectPtr,
+					sizeInBytesRequired);
+			}
 		}
 
 		/* If this was a non-TLH allocation, trigger the hook */
@@ -655,6 +680,15 @@ J9AllocateIndexableObject(J9VMThread *vmThread, J9Class *clazz, uint32_t numberO
 	}
 #endif /* J9VM_GC_THREAD_LOCAL_HEAP */	
 
+	if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_HALT_THREAD_ANY)) {
+		env->saveObjects((omrobjectptr_t)objectPtr);
+		vmThread->javaVM->internalVMFunctions->internalReleaseVMAccess(vmThread);
+		vmThread->javaVM->internalVMFunctions->internalAcquireVMAccess(vmThread);
+		env->restoreObjects((omrobjectptr_t*)&objectPtr);
+	}
+
+	VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 	return objectPtr;
 }
 

runtime/oti/j9consts.h

@@ -197,7 +197,7 @@ extern "C" {
 #define J9_PUBLIC_FLAGS_HALT_THREAD_INSPECTION 0x8000
 #define J9_PUBLIC_FLAGS_HALT_THREAD_HCR 0x10000
 #define J9_PUBLIC_FLAGS_THREAD_PARKED 0x20000
-#define J9_PUBLIC_FLAGS_UNUSED_0x40000 0x40000
+#define J9_PUBLIC_FLAGS_EXCLUSIVE_SET_NOT_SAFE 0x40000
 #define J9_PUBLIC_FLAGS_THREAD_TIMED 0x80000
 #define J9_PUBLIC_FLAGS_UNUSED_0x100000 0x100000
 #define J9_PUBLIC_FLAGS_HALT_THREAD_FOR_CHECKPOINT 0x200000

runtime/vm/VMAccess.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 1991, 2021 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -139,7 +139,9 @@ acquireExclusiveVMAccess(J9VMThread * vmThread)
 	 */
 	if ( ++(vmThread->omrVMThread->exclusiveCount) == 1 ) {
 		omrthread_monitor_enter(vmThread->publicFlagsMutex);
-		VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+		if (J9_ARE_NO_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT)) {
+			VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT | J9_PUBLIC_FLAGS_EXCLUSIVE_SET_NOT_SAFE);
+		}
 		omrthread_monitor_enter(vm->exclusiveAccessMutex);
 		if ( J9_XACCESS_NONE != vm->exclusiveAccessState ) {
 			UDATA reacquireJNICriticalAccess = FALSE;
@@ -503,7 +505,9 @@ void releaseExclusiveVMAccess(J9VMThread * vmThread)
 
 		/* Check the exclusive access queue */
 		omrthread_monitor_enter(vmThread->publicFlagsMutex);
-		VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+		if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_EXCLUSIVE_SET_NOT_SAFE)) {
+			VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT | J9_PUBLIC_FLAGS_EXCLUSIVE_SET_NOT_SAFE);
+		}
 		omrthread_monitor_enter(vm->exclusiveAccessMutex);
 
 		if ( !J9_LINEAR_LINKED_LIST_IS_EMPTY(vm->exclusiveVMAccessQueueHead) ) {
@@ -1243,7 +1247,7 @@ releaseSafePointVMAccess(J9VMThread * vmThread)
  *
  * Note: While the current thread has another thread halted, it must not do anything to modify
  * it's own stack, including the creation of JNI local refs, pushObjectInSpecialFrame, or the
- * running of any java code.
+ * running of any java code or allocating of any java objects.
  */
 
 void
@@ -1256,6 +1260,8 @@ haltThreadForInspection(J9VMThread * currentThread, J9VMThread * vmThread)
 
 	/* Inspecting the current thread does not require any halting */
 	if (currentThread != vmThread) {
+		VM_VMAccess::setPublicFlags(currentThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 		omrthread_monitor_enter(vmThread->publicFlagsMutex);
 
 		/* increment the inspection count but don't try to short circuit -- the thread might not actually be halted yet */
@@ -1313,6 +1319,8 @@ resumeThreadForInspection(J9VMThread * currentThread, J9VMThread * vmThread)
 	/* Inspecting the current thread does not require any halting */
 
 	if (currentThread != vmThread) {
+		VM_VMAccess::clearPublicFlags(currentThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
+
 		/* Ignore resumes for threads which have not been suspended for inspection */
 
 		omrthread_monitor_enter(vmThread->publicFlagsMutex);

runtime/vm/vmhook.c

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 1991, 2017 IBM Corp. and others
+ * Copyright (c) 1991, 2022 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -121,6 +121,13 @@ hookAboutToBootstrapEvent(J9HookInterface** hook, UDATA eventNum, void* voidEven
 	/* these hooks must be reserved by now. Attempt to disable them so that they're in a well-known state after this */
 	(*hookInterface)->J9HookDisable(hookInterface, J9HOOK_VM_MONITOR_CONTENDED_EXIT);
 
+	/* The instrumentable object allocate hook disables safepoint OSR */
+	if ((*hookInterface)->J9HookDisable(hookInterface, J9HOOK_VM_OBJECT_ALLOCATE_INSTRUMENTABLE)) {
+		omrthread_monitor_enter(vm->runtimeFlagsMutex);
+		vm->extendedRuntimeFlags &= ~(UDATA)(J9_EXTENDED_RUNTIME_OSR_SAFE_POINT| J9_EXTENDED_RUNTIME_OSR_SAFE_POINT_FV);
+		omrthread_monitor_exit(vm->runtimeFlagsMutex);
+	}
+
 	if ((*hookInterface)->J9HookDisable(hookInterface, J9HOOK_VM_METHOD_ENTER)
 		|| (*hookInterface)->J9HookDisable(hookInterface, J9HOOK_VM_METHOD_RETURN)
 		|| (*hookInterface)->J9HookDisable(hookInterface, J9HOOK_VM_FRAME_POP)