Check if current phase concurrent when trying to yield

When we consider yielding in Concurrent Scavenger (prematurely
terminating scanning loop, while in concurrent phase, due to external
Exclusive Access Request being raised), we can now rely on explicit flag
that tells us we are really in concurrent phase. Yielding while in STW
phases is meaningless.

Previously we checked if we were not in STW mode by checking that
exclusive access was in 'pending' state, but exclusive VM access
actually can be pending even state is already 'exclusive'. This is true
when exclusive VM access was requested by external thread. In this case,
we could incorrectly conclude we are not in concurrent mode, and never
yield.

Signed-off-by: Aleksandar Micic <amicic@ca.ibm.com>

example/glue/ScavengerDelegate.hpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019, 2020 IBM Corp. and others
+ * Copyright (c) 2019, 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
@@ -242,8 +242,6 @@ class MM_ScavengerDelegate : public MM_BaseVirtual {
 	 * Fixup should update slots to point to the forwarded version of the object and/or remove self forwarded bit in the object itself.
 	 */
 	void fixupIndirectObjectSlots(MM_EnvironmentStandard *env, omrobjectptr_t objectPtr);
-
-	bool shouldYield() { return false; }
 #endif /* OMR_GC_CONCURRENT_SCAVENGER */
 
 	bool initialize(MM_EnvironmentBase* env) { return true; }

gc/base/standard/Scavenger.cpp

@@ -3681,10 +3681,11 @@ MM_Scavenger::checkAndSetShouldYieldFlag(MM_EnvironmentStandard *env) {
 	 * Main info if we should yield comes from exclusive VM access request being broadcasted to this thread (isExclusiveAccessRequestWaiting())
 	 * But since that request in the thread is not cleared even when implicit main GC thread enters STW phase, and since this yield check is invoked
 	 * in common code that can run both during STW and concurrent phase, we have to additionally check we are indeed in concurrent phase before deciding to yield.
-	 * Most of the time we could rely on being in 'concurrent_phase_scan' but it's more reliable to actually check if exclusive access
-	 * is indeed being requested (hence shouldYield() call to delegate, too).
 	 */
-	if (!_shouldYield && env->isExclusiveAccessRequestWaiting() && _delegate.shouldYield()) {
+
+	if (isCurrentPhaseConcurrent() && env->isExclusiveAccessRequestWaiting() && !_shouldYield) {
+		/* If we are yielding we must be working concurrently, so GC better not have exclusive VM access. We can really only assert it for the current thread */
+		Assert_MM_true(0 == env->getOmrVMThread()->exclusiveCount);
 		_shouldYield = true;
 	}
 	return _shouldYield;
@@ -5648,6 +5649,8 @@ MM_Scavenger::mainThreadConcurrentCollect(MM_EnvironmentBase *env)
 		clearIncrementGCStats(env, false);
 
 		_currentPhaseConcurrent = true;
+		/* We claim to work concurrently. GC better not have exclusive VM access. We can really only assert it for the current thread */
+		Assert_MM_true(0 == env->getOmrVMThread()->exclusiveCount);
 
 		MM_ConcurrentScavengeTask scavengeTask(env, _dispatcher, this, MM_ConcurrentScavengeTask::SCAVENGE_SCAN, env->_cycleState);
 		/* Concurrent background task will run with different (typically lower) number of threads. */

gc/base/standard/Scavenger.hpp

@@ -765,11 +765,6 @@ class MM_Scavenger : public MM_Collector
 		return concurrent_phase_idle != _concurrentPhase;
 	}
 
-	/* TODO: remove once downstream projects start using isConcurrentCycleInProgress/isCurrentPhaseConcurrent */
-	bool isConcurrentInProgress() {
-		return concurrent_phase_idle != _concurrentPhase;
-	}
-
 	bool isMutatorThreadInSyncWithCycle(MM_EnvironmentBase *env) {
 		return (env->_concurrentScavengerSwitchCount == _concurrentScavengerSwitchCount);
 	}