Skip to content

Commit 13ebc10

Browse files
committed
Fix missing concurrent continuation scanning issue
The issue is caused by concurrent scavenger scanning and concurrent marking scanning overlap for the same continuation Object, during concurrent continuation scanning, the current synchronization control would block the continuation mounting and ignore another concurrent scanning, but the concurrent scavenger scanning and concurrent marking are irrelevant, ignore another could cause missing scanning and the related "live" object is recycled. - updated J9VMContinuation->state, use two low bits for recording concurrentScanning(bit0 for concurrentScanningLocal and bit1 for concurrentScanningGlobal) instead of only one low bit. - pass flag isConcurrentGC and flag isGlobalGC for GC_VMThreadStackSlotIterator::scanSlots(). - handle J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_LOCAL and J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_GLOBAL independently - only both J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_LOCAL and J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_GLOBAL bits has been cleared we can notify blocked the continuation mounting thread. Signed-off-by: Lin Hu <linhu@ca.ibm.com>
1 parent 056acd0 commit 13ebc10

17 files changed

+133
-65
lines changed

runtime/gc_base/GCExtensions.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ MM_GCExtensions::releaseNativesForContinuationObject(MM_EnvironmentBase* env, j9
320320
}
321321

322322
bool
323-
MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr)
323+
MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr, bool isGlobalGC)
324324
{
325325
bool needScan = false;
326326
#if JAVA_SPEC_VERSION >= 19
@@ -335,12 +335,14 @@ MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9obj
335335
*
336336
* For fully STW GCs, there is no harm to scan them, but it's a waste of time since they are scanned during root scanning already.
337337
*
338-
* We don't scan currently scanned either - one scan is enough.
338+
* We don't scan currently scanned for the same collector either - one scan is enough for the same collector, but there could be concurrent scavenger(local collector) and concurrent marking(global collector) overlapping,
339+
* they are irrelevant and both are concurrent, we handle them independently and separately, they are not blocked or ignored each other.
340+
*
339341
* we don't scan the continuation object before started and after finished - java stack does not exist.
340342
*/
341343
if (started && !finished) {
342344
Assert_MM_true(NULL != continuation);
343-
needScan = !VM_VMHelpers::isContinuationMountedOrConcurrentlyScanned(continuation);
345+
needScan = !VM_VMHelpers::isContinuationMountedOrConcurrentlyScanned(continuation, isGlobalGC);
344346
}
345347
#endif /* JAVA_SPEC_VERSION >= 19 */
346348
return needScan;

runtime/gc_base/GCExtensions.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,13 @@ class MM_GCExtensions : public MM_GCExtensionsBase {
303303
* Check if we need to scan the java stack for the Continuation Object
304304
* Used during main scan phase of GC (object graph traversal) or heap object iteration (in sliding compact).
305305
* Not meant to be used during root scanning (neither strong roots nor weak roots)!
306+
*
306307
* @param[in] vmThread the current J9VMThread
307308
* @param[in] continuationObject the continuation object
309+
* @param[in] isGlobalGC
308310
* @return true if we need to scan the java stack
309311
*/
310-
static bool needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr);
312+
static bool needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr, bool isGlobalGC);
311313

312314
/**
313315
* Create a GCExtensions object

runtime/gc_glue_java/CompactSchemeFixupObject.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,15 @@ MM_CompactSchemeFixupObject::fixupContinuationNativeSlots(MM_EnvironmentStandard
7575
* mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
7676
* (hence passing true for scanOnlyUnmounted parameter).
7777
*/
78-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
78+
const bool isGlobalGC = true;
79+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
7980
StackIteratorData4CompactSchemeFixupObject localData;
8081
localData.compactSchemeFixupObject = this;
8182
localData.env = env;
8283
localData.fromObject = objectPtr;
84+
const bool isConcurrentGC = false;
8385

84-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCompactScheme, false, false);
86+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCompactScheme, false, false, isConcurrentGC, isGlobalGC);
8587
}
8688
}
8789

runtime/gc_glue_java/HeapWalkerDelegate.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,17 @@ MM_HeapWalkerDelegate::doContinuationNativeSlots(MM_EnvironmentBase *env, omrobj
6060
{
6161
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
6262

63-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
63+
const bool isGlobalGC = true;
64+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
6465
StackIteratorData4HeapWalker localData;
6566
localData.heapWalker = _heapWalker;
6667
localData.env = env;
6768
localData.fromObject = objectPtr;
6869
localData.function = function;
6970
localData.userData = userData;
7071
/* so far there is no case we need ClassWalk for heapwalker, so we set stackFrameClassWalkNeeded = false */
71-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForHeapWalker, false, false);
72+
const bool isConcurrentGC = false;
73+
74+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForHeapWalker, false, false, isConcurrentGC, isGlobalGC);
7275
}
7376
}

runtime/gc_glue_java/MarkingDelegate.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,8 @@ void
261261
MM_MarkingDelegate::scanContinuationNativeSlots(MM_EnvironmentBase *env, omrobjectptr_t objectPtr)
262262
{
263263
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
264-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
264+
const bool isGlobalGC = true;
265+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
265266
StackIteratorData4MarkingDelegate localData;
266267
localData.markingDelegate = this;
267268
localData.env = env;
@@ -271,10 +272,10 @@ MM_MarkingDelegate::scanContinuationNativeSlots(MM_EnvironmentBase *env, omrobje
271272
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
272273
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
273274
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
274-
275275
/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
276-
bool syncWithContinuationMounting = J9_ARE_ANY_BITS_SET(currentThread->privateFlags, J9_PRIVATE_FLAGS_CONCURRENT_MARK_ACTIVE);
277-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForMarkingDelegate, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
276+
bool isConcurrentGC = J9_ARE_ANY_BITS_SET(currentThread->privateFlags, J9_PRIVATE_FLAGS_CONCURRENT_MARK_ACTIVE);
277+
278+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForMarkingDelegate, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
278279
}
279280
}
280281

runtime/gc_glue_java/MetronomeDelegate.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -1647,7 +1647,8 @@ void
16471647
MM_MetronomeDelegate::scanContinuationNativeSlots(MM_EnvironmentRealtime *env, J9Object *objectPtr)
16481648
{
16491649
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
1650-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
1650+
const bool isGlobalGC = true;
1651+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
16511652
StackIteratorData4RealtimeMarkingScheme localData;
16521653
localData.realtimeMarkingScheme = _markingScheme;
16531654
localData.env = env;
@@ -1657,11 +1658,10 @@ MM_MetronomeDelegate::scanContinuationNativeSlots(MM_EnvironmentRealtime *env, J
16571658
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
16581659
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
16591660
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
1660-
16611661
/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
1662-
bool syncWithContinuationMounting = _realtimeGC->isCollectorConcurrentTracing();
1662+
bool isConcurrentGC = _realtimeGC->isCollectorConcurrentTracing();
16631663

1664-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForRealtimeGC, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
1664+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForRealtimeGC, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
16651665
}
16661666
}
16671667

runtime/gc_glue_java/ScavengerDelegate.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,17 @@ MM_ScavengerDelegate::scanContinuationNativeSlots(MM_EnvironmentStandard *env, o
345345
bool shouldRemember = false;
346346

347347
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
348-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
348+
const bool isGlobalGC = false;
349+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
349350
StackIteratorData4Scavenge localData;
350351
localData.scavengerDelegate = this;
351352
localData.env = env;
352353
localData.reason = reason;
353354
localData.shouldRemember = &shouldRemember;
354355
/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
355-
bool syncWithContinuationMounting = _extensions->isConcurrentScavengerInProgress();
356+
bool isConcurrentGC = _extensions->isConcurrentScavengerInProgress();
356357

357-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForScavenge, false, false, syncWithContinuationMounting);
358+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForScavenge, false, false, isConcurrentGC, isGlobalGC);
358359
}
359360
return shouldRemember;
360361
}

runtime/gc_structs/VMThreadStackSlotIterator.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
/*******************************************************************************
3-
* Copyright (c) 1991, 2022 IBM Corp. and others
3+
* Copyright (c) 1991, 2023 IBM Corp. and others
44
*
55
* This program and the accompanying materials are made available under
66
* the terms of the Eclipse Public License 2.0 which accompanies this
@@ -137,13 +137,14 @@ GC_VMThreadStackSlotIterator::scanSlots(
137137
J9MODRON_OSLOTITERATOR *oSlotIterator,
138138
bool includeStackFrameClassReferences,
139139
bool trackVisibleFrameDepth,
140-
bool syncWithContinuationMounting
140+
bool isConcurrentGC,
141+
bool isGlobalGC
141142
)
142143
{
143144
J9StackWalkState stackWalkState;
144145

145146
initializeStackWalkState(&stackWalkState, vmThread, userData, oSlotIterator, includeStackFrameClassReferences, trackVisibleFrameDepth);
146-
VM_VMHelpers::walkContinuationStackFramesWrapper(vmThread, continuationObjectPtr, &stackWalkState, syncWithContinuationMounting);
147+
VM_VMHelpers::walkContinuationStackFramesWrapper(vmThread, continuationObjectPtr, &stackWalkState, isConcurrentGC, isGlobalGC);
147148
}
148149

149150
#if JAVA_SPEC_VERSION >= 19

runtime/gc_structs/VMThreadStackSlotIterator.hpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
/*******************************************************************************
3-
* Copyright (c) 1991, 2022 IBM Corp. and others
3+
* Copyright (c) 1991, 2023 IBM Corp. and others
44
*
55
* This program and the accompanying materials are made available under
66
* the terms of the Eclipse Public License 2.0 which accompanies this
@@ -67,7 +67,8 @@ class GC_VMThreadStackSlotIterator
6767
J9MODRON_OSLOTITERATOR *oSlotIterator,
6868
bool includeStackFrameClassReferences,
6969
bool trackVisibleFrameDepth,
70-
bool syncWithContinuationMounting = false);
70+
bool isConcurrentGC,
71+
bool isGlobalGC);
7172

7273
#if JAVA_SPEC_VERSION >= 19
7374
static void scanSlots(

runtime/gc_vlhgc/CopyForwardScheme.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -2322,7 +2322,8 @@ MMINLINE void
23222322
MM_CopyForwardScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, MM_AllocationContextTarok *reservingContext, J9Object *objectPtr, ScanReason reason)
23232323
{
23242324
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
2325-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
2325+
const bool isGlobalGC = false;
2326+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
23262327
StackIteratorData4CopyForward localData;
23272328
localData.copyForwardScheme = this;
23282329
localData.env = env;
@@ -2332,7 +2333,9 @@ MM_CopyForwardScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, MM_A
23322333
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
23332334
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
23342335
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
2335-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCopyForwardScheme, stackFrameClassWalkNeeded, false);
2336+
const bool isConcurrentGC = false;
2337+
2338+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCopyForwardScheme, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
23362339
}
23372340
}
23382341

runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,16 @@ bool MM_GlobalMarkCardScrubber::scrubContinuationNativeSlots(MM_EnvironmentVLHGC
192192
{
193193
bool doScrub = true;
194194
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
195-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
195+
const bool isGlobalGC = true;
196+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
196197
StackIteratorData4GlobalMarkCardScrubber localData;
197198
localData.globalMarkCardScrubber = this;
198199
localData.env = env;
199200
localData.doScrub = &doScrub;
200201
localData.fromObject = objectPtr;
202+
const bool isConcurrentGC = false;
201203

202-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkCardScrubber, false, false);
204+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkCardScrubber, false, false, isConcurrentGC, isGlobalGC);
203205
}
204206
return doScrub;
205207
}

runtime/gc_vlhgc/GlobalMarkingScheme.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,8 @@ void
792792
MM_GlobalMarkingScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, J9Object *objectPtr, ScanReason reason)
793793
{
794794
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
795-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
795+
const bool isGlobalGC = true;
796+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
796797
StackIteratorData4GlobalMarkingScheme localData;
797798
localData.globalMarkingScheme = this;
798799
localData.env = env;
@@ -801,11 +802,10 @@ MM_GlobalMarkingScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, J9
801802
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
802803
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
803804
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
804-
805805
/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
806-
bool syncWithContinuationMounting = (MM_VLHGCIncrementStats::mark_concurrent == static_cast<MM_CycleStateVLHGC*>(env->_cycleState)->_vlhgcIncrementStats._globalMarkIncrementType);
806+
bool isConcurrentGC = (MM_VLHGCIncrementStats::mark_concurrent == static_cast<MM_CycleStateVLHGC*>(env->_cycleState)->_vlhgcIncrementStats._globalMarkIncrementType);
807807

808-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkingScheme, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
808+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkingScheme, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
809809
}
810810
}
811811

runtime/gc_vlhgc/WriteOnceCompactor.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -1240,13 +1240,15 @@ MM_WriteOnceCompactor::fixupContinuationNativeSlots(MM_EnvironmentVLHGC* env, J9
12401240
* mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
12411241
* (hence passing true for scanOnlyUnmounted parameter).
12421242
*/
1243-
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
1243+
const bool isGlobalGC = MM_CycleState::CT_GLOBAL_GARBAGE_COLLECTION == env->_cycleState->_collectionType;
1244+
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
12441245
StackIteratorData4WriteOnceCompactor localData;
12451246
localData.writeOnceCompactor = this;
12461247
localData.env = env;
12471248
localData.fromObject = objectPtr;
1249+
const bool isConcurrentGC = false;
12481250

1249-
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForWriteOnceCompactor, false, false);
1251+
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForWriteOnceCompactor, false, false, isConcurrentGC, isGlobalGC);
12501252
}
12511253
}
12521254

0 commit comments

Comments
 (0)