Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix halted thread being allowed to continue execution #15201

Merged
merged 1 commit into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions runtime/gc_base/IdleGCManager.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -33,6 +32,7 @@
#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,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);
}

Expand Down
20 changes: 17 additions & 3 deletions runtime/gc_base/modronapi.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -48,6 +47,7 @@
#include "MemoryPoolLargeObjects.hpp"
#include "VMInterface.hpp"
#include "VMThreadListIterator.hpp"
#include "VMAccess.hpp"

extern "C" {

Expand Down Expand Up @@ -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;
}

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

Expand Down Expand Up @@ -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) */
4 changes: 3 additions & 1 deletion runtime/gc_glue_java/ConcurrentMarkingDelegate.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions runtime/gc_glue_java/EnvironmentDelegate.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
56 changes: 45 additions & 11 deletions runtime/gc_modron_startup/mgcalloc.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
gacholio marked this conversation as resolved.
Show resolved Hide resolved
ALWAYS_TRIGGER_J9HOOK_VM_OBJECT_ALLOCATE(
vmThread->javaVM->hookInterface,
vmThread,
objectPtr,
sizeInBytesRequired);
}
}

if( !mixedOAM.getAllocateDescription()->isCompletedFromTlh()) {
Expand Down Expand Up @@ -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);

gacholio marked this conversation as resolved.
Show resolved Hide resolved
return objectPtr;
}

Expand All @@ -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));
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}

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_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
Expand Down
16 changes: 12 additions & 4 deletions runtime/vm/VMAccess.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) ) {
Expand Down Expand Up @@ -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
Expand All @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 8 additions & 1 deletion runtime/vm/vmhook.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand Down