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

Code before:
        goto retry_for_debugger;
    }
#endif // DEBUGGING_SUPPORTED

    GC_ON_TRANSITIONS(gcOnTransitions);

    FireEtwGCSuspendEEEnd_V1(GetClrInstanceId());

#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();
    _ASSERTE(pThread != NULL);

    // 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

Review comment (bug):
The assertion `_ASSERTE(pThread != NULL)` on line 5735 conflicts with the subsequent null check on line 5739. If the assertion is correct and `pThread` can never be NULL, then the null checks on lines 5739 and 5745 are redundant and should be removed. However, if there are cases where `GetThreadAsyncSafe()` can return NULL (e.g., if the thread hasn't been registered yet), then the assertion is incorrect and should be changed to a null check with early return.

Given that this is called from an async signal handler where the thread might not be fully initialized, the safer approach would be to remove the assertion and handle the NULL case gracefully:
```c
Thread *pThread = GetThreadAsyncSafe();
if (pThread == NULL)
{
    return FALSE;
}
```
```suggestion
    if (pThread == NULL)
    {
        return FALSE;
    }
```

Code after:
        goto retry_for_debugger;
    }
#endif // DEBUGGING_SUPPORTED

    GC_ON_TRANSITIONS(gcOnTransitions);

    FireEtwGCSuspendEEEnd_V1(GetClrInstanceId());

#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.