Repository: dotnet/runtime
File: src/coreclr/vm/threadsuspend.cpp (C++)

Code before:

#ifdef TIME_SUSPEND
    g_SuspendStatistics.EndSuspend(reason == SUSPEND_FOR_GC || reason == SUSPEND_FOR_GC_PREP);
#endif //TIME_SUSPEND
    ThreadSuspend::s_fSuspended = true;
}

#ifdef FEATURE_THREAD_ACTIVATION

// This function is called by a thread activation to check if the specified instruction pointer
// is in a function where we can safely handle an activation.
// WARNING: This method is called by suspension while one thread is interrupted
//          in a random location, possibly holding random locks.
//          It is unsafe to use blocking APIs or allocate in this method.
BOOL CheckActivationSafePoint(SIZE_T ip)
{
    Thread *pThread = GetThreadAsyncSafe();

    // The criteria for safe activation is to be running managed code.
    // Also we are not interested in handling interruption if we are already in preemptive mode nor if we are single stepping
    BOOL isActivationSafePoint = pThread != NULL &&
        (pThread->m_StateNC & Thread::TSNC_DebuggerIsStepping) == 0 &&
        pThread->PreemptiveGCDisabled() &&
        (ExecutionManager::GetScanFlags(pThread) != ExecutionManager::ScanReaderLock) &&
        ExecutionManager::IsManagedCodeNoLock(ip);

    if (!isActivationSafePoint && pThread != NULL)
    {
        pThread->m_hasPendingActivation = false;
    }

    return isActivationSafePoint;
}

// This function is called when thread suspension is pending. It tries to ensure that the current
// thread is taken to a GC-safe place as quickly as possible. It does this by doing
// one of the following:
//
//     - If the thread is in native code or preemptive GC is not disabled, there's
//       nothing to do, so we return.
//
//     - If the thread is in interruptible managed code, we will push a frame that
//       has information about the context that was interrupted and then switch to
//       preemptive GC mode so that the pending GC can proceed, and then switch back.
//
//     - If the thread is in uninterruptible managed code, we will patch the return
//       address to take the thread to the appropriate stub (based on the return
//       type of the method) which will then handle preparing the thread for GC.
//
void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
{

Review comment (bug):
The reverted code is checking `pThread` for NULL on line 5743, but if `pThread` is NULL on line 5734, accessing `pThread->m_hasPendingActivation` on line 5745 will cause a null pointer dereference. The condition on line 5743 should remain `if (!isActivationSafePoint && pThread != NULL)` to avoid crashes when pThread is NULL.
```suggestion
    if (!isActivationSafePoint && pThread != NULL)
```

Code after:

#ifdef TIME_SUSPEND
    g_SuspendStatistics.EndSuspend(reason == SUSPEND_FOR_GC || reason == SUSPEND_FOR_GC_PREP);
#endif //TIME_SUSPEND
    ThreadSuspend::s_fSuspended = true;
}

#ifdef FEATURE_THREAD_ACTIVATION

// This function is called by a thread activation to check if the specified instruction pointer
// is in a function where we can safely handle an activation.
// WARNING: This method is called by suspension while one thread is interrupted
//          in a random location, possibly holding random locks.
//          It is unsafe to use blocking APIs or allocate in this method.
BOOL CheckActivationSafePoint(SIZE_T ip)
{
    Thread *pThread = GetThreadNULLOk();

    // The criteria for safe activation is to be running managed code.
    // Also we are not interested in handling interruption if we are already in preemptive mode nor if we are single stepping
    BOOL isActivationSafePoint = pThread != NULL &&
        (pThread->m_StateNC & Thread::TSNC_DebuggerIsStepping) == 0 &&
        pThread->PreemptiveGCDisabled() &&
        ExecutionManager::IsManagedCode(ip);

    if (!isActivationSafePoint)
    {
        pThread->m_hasPendingActivation = false;
    }

    return isActivationSafePoint;
}

// This function is called when thread suspension is pending. It tries to ensure that the current
// thread is taken to a GC-safe place as quickly as possible. It does this by doing
// one of the following:
//
//     - If the thread is in native code or preemptive GC is not disabled, there's
//       nothing to do, so we return.
//
//     - If the thread is in interruptible managed code, we will push a frame that
//       has information about the context that was interrupted and then switch to
//       preemptive GC mode so that the pending GC can proceed, and then switch back.
//
//     - If the thread is in uninterruptible managed code, we will patch the return
//       address to take the thread to the appropriate stub (based on the return
//       type of the method) which will then handle preparing the thread for GC.
//
void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
{
    struct AutoClearPendingThreadActivation