Skip to content

Commit

Permalink
Merge pull request #15226 from eclipse-openj9/revert-15201-safe
Browse files Browse the repository at this point in the history
Revert "Fix halted thread being allowed to continue execution"
  • Loading branch information
pshipton authored Jun 4, 2022
2 parents c4d39a3 + a1f1df2 commit eecd210
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 107 deletions.
6 changes: 2 additions & 4 deletions runtime/gc_base/IdleGCManager.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2020 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
Expand Down Expand Up @@ -32,7 +33,6 @@
#include "GCExtensions.hpp"
#include "OMRVMInterface.hpp"
#include "Heap.hpp"
#include "VMAccess.hpp"

MM_IdleGCManager *
MM_IdleGCManager::newInstance(MM_EnvironmentBase* env)
Expand Down Expand Up @@ -81,9 +81,7 @@ 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);
}

Expand Down
20 changes: 3 additions & 17 deletions runtime/gc_base/modronapi.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2020 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
Expand Down Expand Up @@ -47,7 +48,6 @@
#include "MemoryPoolLargeObjects.hpp"
#include "VMInterface.hpp"
#include "VMThreadListIterator.hpp"
#include "VMAccess.hpp"

extern "C" {

Expand Down Expand Up @@ -86,15 +86,8 @@ 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;
}

Expand All @@ -103,14 +96,7 @@ 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;
}
Expand Down
16 changes: 0 additions & 16 deletions runtime/gc_glue_java/ConcurrentMarkingDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "HeapRegionIteratorStandard.hpp"
#include "ReferenceObjectList.hpp"
#include "StackSlotValidator.hpp"
#include "VMAccess.hpp"
#include "VMInterface.hpp"
#include "VMThreadListIterator.hpp"

Expand Down Expand Up @@ -451,19 +450,4 @@ 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) */
4 changes: 1 addition & 3 deletions runtime/gc_glue_java/ConcurrentMarkingDelegate.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2021 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
Expand Down Expand Up @@ -249,8 +249,6 @@ 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.
Expand Down
4 changes: 3 additions & 1 deletion runtime/gc_glue_java/EnvironmentDelegate.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2017, 2022 IBM Corp. and others
* Copyright (c) 2017, 2021 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
Expand Down Expand Up @@ -234,13 +234,15 @@ 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
Expand Down
56 changes: 11 additions & 45 deletions runtime/gc_modron_startup/mgcalloc.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2021 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
Expand Down Expand Up @@ -387,8 +387,6 @@ 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;
Expand Down Expand Up @@ -456,17 +454,11 @@ J9AllocateObject(J9VMThread *vmThread, J9Class *clazz, uintptr_t allocateFlags)
objectPtr,
sizeInBytesRequired);
} else {
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);
}
TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
vmThread->javaVM->hookInterface,
vmThread,
objectPtr,
sizeInBytesRequired);
}

if( !mixedOAM.getAllocateDescription()->isCompletedFromTlh()) {
Expand Down Expand Up @@ -524,15 +516,6 @@ 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;
}

Expand All @@ -550,8 +533,6 @@ 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));
Expand Down Expand Up @@ -608,17 +589,11 @@ J9AllocateIndexableObject(J9VMThread *vmThread, J9Class *clazz, uint32_t numberO
objectPtr,
sizeInBytesRequired);
} else {
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);
}
TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
vmThread->javaVM->hookInterface,
vmThread,
objectPtr,
sizeInBytesRequired);
}

/* If this was a non-TLH allocation, trigger the hook */
Expand Down Expand Up @@ -680,15 +655,6 @@ 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;
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/oti/j9consts.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_EXCLUSIVE_SET_NOT_SAFE 0x40000
#define J9_PUBLIC_FLAGS_UNUSED_0x40000 0x40000
#define J9_PUBLIC_FLAGS_THREAD_TIMED 0x80000
#define J9_PUBLIC_FLAGS_UNUSED_0x100000 0x100000
#define J9_PUBLIC_FLAGS_HALT_THREAD_FOR_CHECKPOINT 0x200000
Expand Down
16 changes: 4 additions & 12 deletions runtime/vm/VMAccess.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2021 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
Expand Down Expand Up @@ -139,9 +139,7 @@ acquireExclusiveVMAccess(J9VMThread * vmThread)
*/
if ( ++(vmThread->omrVMThread->exclusiveCount) == 1 ) {
omrthread_monitor_enter(vmThread->publicFlagsMutex);
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);
}
VM_VMAccess::setPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
omrthread_monitor_enter(vm->exclusiveAccessMutex);
if ( J9_XACCESS_NONE != vm->exclusiveAccessState ) {
UDATA reacquireJNICriticalAccess = FALSE;
Expand Down Expand Up @@ -505,9 +503,7 @@ void releaseExclusiveVMAccess(J9VMThread * vmThread)

/* Check the exclusive access queue */
omrthread_monitor_enter(vmThread->publicFlagsMutex);
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);
}
VM_VMAccess::clearPublicFlags(vmThread, J9_PUBLIC_FLAGS_NOT_AT_SAFE_POINT);
omrthread_monitor_enter(vm->exclusiveAccessMutex);

if ( !J9_LINEAR_LINKED_LIST_IS_EMPTY(vm->exclusiveVMAccessQueueHead) ) {
Expand Down Expand Up @@ -1247,7 +1243,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 or allocating of any java objects.
* running of any java code.
*/

void
Expand All @@ -1260,8 +1256,6 @@ 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 */
Expand Down Expand Up @@ -1319,8 +1313,6 @@ 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);
Expand Down
9 changes: 1 addition & 8 deletions runtime/vm/vmhook.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2017 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
Expand Down Expand Up @@ -121,13 +121,6 @@ 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)
Expand Down

0 comments on commit eecd210

Please sign in to comment.