Skip to content

Commit

Permalink
Convert WaitHandle FCalls to QCalls (#107488)
Browse files Browse the repository at this point in the history
  • Loading branch information
AaronRobinsonMSFT committed Sep 8, 2024
1 parent b523ec5 commit 10f6c4c
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public bool WaitCore(int timeoutMs)
return waitResult == WaitHandle.WaitSuccess;
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "WaitHandle_CorWaitOnePrioritizedNative")]
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "WaitHandle_WaitOnePrioritized")]
private static partial int WaitNative(SafeWaitHandle handle, int timeoutMs);

private void ReleaseCore(int count)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,18 @@ namespace System.Threading
{
public abstract partial class WaitHandle
{
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, bool useTrivialWaits);
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "WaitHandle_WaitOneCore")]
private static partial int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, [MarshalAs(UnmanagedType.Bool)] bool useTrivialWaits);

private static unsafe int WaitMultipleIgnoringSyncContextCore(ReadOnlySpan<IntPtr> waitHandles, bool waitAll, int millisecondsTimeout)
{
fixed (IntPtr* pWaitHandles = &MemoryMarshal.GetReference(waitHandles))
{
return WaitMultipleIgnoringSyncContext(pWaitHandles, waitHandles.Length, waitAll, millisecondsTimeout);
}
}
=> WaitMultipleIgnoringSyncContext(waitHandles, waitHandles.Length, waitAll, millisecondsTimeout);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe int WaitMultipleIgnoringSyncContext(IntPtr* waitHandles, int numHandles, bool waitAll, int millisecondsTimeout);
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "WaitHandle_WaitMultipleIgnoringSyncContext")]
private static partial int WaitMultipleIgnoringSyncContext(ReadOnlySpan<IntPtr> waitHandles, int numHandles, [MarshalAs(UnmanagedType.Bool)] bool waitAll, int millisecondsTimeout);

private static int SignalAndWaitCore(IntPtr waitHandleToSignal, IntPtr waitHandleToWaitOn, int millisecondsTimeout)
{
int ret = SignalAndWaitNative(waitHandleToSignal, waitHandleToWaitOn, millisecondsTimeout);
int ret = SignalAndWait(waitHandleToSignal, waitHandleToWaitOn, millisecondsTimeout);

if (ret == Interop.Errors.ERROR_TOO_MANY_POSTS)
{
Expand All @@ -34,7 +29,7 @@ private static int SignalAndWaitCore(IntPtr waitHandleToSignal, IntPtr waitHandl
return ret;
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern int SignalAndWaitNative(IntPtr waitHandleToSignal, IntPtr waitHandleToWaitOn, int millisecondsTimeout);
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "WaitHandle_SignalAndWait")]
private static partial int SignalAndWait(IntPtr waitHandleToSignal, IntPtr waitHandleToWaitOn, int millisecondsTimeout);
}
}
99 changes: 43 additions & 56 deletions src/coreclr/vm/comwaithandle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,106 +11,93 @@
**
===========================================================*/
#include "common.h"
#include "object.h"
#include "field.h"
#include "excep.h"
#include "comwaithandle.h"

FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, FC_BOOL_ARG useTrivialWaits)
extern "C" INT32 QCALLTYPE WaitHandle_WaitOneCore(HANDLE handle, INT32 timeout, BOOL useTrivialWaits)
{
FCALL_CONTRACT;
QCALL_CONTRACT;

INT32 retVal = 0;
HELPER_METHOD_FRAME_BEGIN_RET_0();

BEGIN_QCALL;

_ASSERTE(handle != 0);
_ASSERTE(handle != INVALID_HANDLE_VALUE);

Thread* pThread = GET_THREAD();

WaitMode waitMode = (WaitMode)((!FC_ACCESS_BOOL(useTrivialWaits) ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx);
WaitMode waitMode = (WaitMode)((!useTrivialWaits ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx);
retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, waitMode);

HELPER_METHOD_FRAME_END();
END_QCALL;
return retVal;
}
FCIMPLEND

#ifdef TARGET_UNIX
extern "C" INT32 QCALLTYPE WaitHandle_CorWaitOnePrioritizedNative(HANDLE handle, INT32 timeoutMs)
extern "C" INT32 QCALLTYPE WaitHandle_WaitMultipleIgnoringSyncContext(HANDLE *handleArray, INT32 numHandles, BOOL waitForAll, INT32 timeout)
{
QCALL_CONTRACT;

DWORD result = WAIT_FAILED;

BEGIN_QCALL;

_ASSERTE(handle != NULL);
_ASSERTE(handle != INVALID_HANDLE_VALUE);

result = PAL_WaitForSingleObjectPrioritized(handle, timeoutMs);

END_QCALL;
return (INT32)result;
}
#endif

FCIMPL4(INT32, WaitHandleNative::CorWaitMultipleNative, HANDLE *handleArray, INT32 numHandles, FC_BOOL_ARG waitForAll, INT32 timeout)
{
FCALL_CONTRACT;

INT32 ret = 0;
HELPER_METHOD_FRAME_BEGIN_RET_0();
BEGIN_QCALL;

Thread * pThread = GET_THREAD();

#ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
// There are some issues with wait-all from an STA thread
// - https://github.com/dotnet/runtime/issues/10243#issuecomment-385117537
if (FC_ACCESS_BOOL(waitForAll) && numHandles > 1 && pThread->GetApartment() == Thread::AS_InSTA)
if (waitForAll && numHandles > 1 && pThread->GetApartment() == Thread::AS_InSTA)
{
COMPlusThrow(kNotSupportedException, W("NotSupported_WaitAllSTAThread"));
}
#endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT

ret = pThread->DoAppropriateWait(numHandles, handleArray, FC_ACCESS_BOOL(waitForAll), timeout, (WaitMode)(WaitMode_Alertable | WaitMode_IgnoreSyncCtx));
ret = pThread->DoAppropriateWait(numHandles, handleArray, waitForAll, timeout, (WaitMode)(WaitMode_Alertable | WaitMode_IgnoreSyncCtx));

HELPER_METHOD_FRAME_END();
END_QCALL;
return ret;
}
FCIMPLEND

FCIMPL3(INT32, WaitHandleNative::CorSignalAndWaitOneNative, HANDLE waitHandleSignalUNSAFE, HANDLE waitHandleWaitUNSAFE, INT32 timeout)
extern "C" INT32 QCALLTYPE WaitHandle_SignalAndWait(HANDLE waitHandleSignal, HANDLE waitHandleWait, INT32 timeout)
{
FCALL_CONTRACT;
QCALL_CONTRACT;

INT32 retVal = 0;
INT32 retVal = (DWORD)-1;

HELPER_METHOD_FRAME_BEGIN_RET_0();
BEGIN_QCALL;

_ASSERTE(waitHandleSignalUNSAFE != 0);
_ASSERTE(waitHandleWaitUNSAFE != 0);
_ASSERTE(waitHandleSignal != 0);
_ASSERTE(waitHandleWait != 0);

Thread* pThread = GET_THREAD();

#ifdef FEATURE_COMINTEROP
if (pThread->GetApartment() == Thread::AS_InSTA) {
COMPlusThrow(kNotSupportedException, W("NotSupported_SignalAndWaitSTAThread")); //<TODO> Change this message
}
#endif

DWORD res = (DWORD) -1;

HANDLE handles[2];
handles[0] = waitHandleSignalUNSAFE;
handles[1] = waitHandleWaitUNSAFE;
#ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
if (pThread->GetApartment() == Thread::AS_InSTA)
{
res = pThread->DoSignalAndWait(handles, timeout, TRUE /*alertable*/);
COMPlusThrow(kNotSupportedException, W("NotSupported_SignalAndWaitSTAThread"));
}
#endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT

retVal = res;
HANDLE handles[] = { waitHandleSignal, waitHandleWait };
retVal = pThread->DoSignalAndWait(handles, timeout, TRUE /*alertable*/);

HELPER_METHOD_FRAME_END();
END_QCALL;
return retVal;
}
FCIMPLEND

#ifdef TARGET_UNIX
extern "C" INT32 QCALLTYPE WaitHandle_WaitOnePrioritized(HANDLE handle, INT32 timeoutMs)
{
QCALL_CONTRACT;

DWORD result = WAIT_FAILED;

BEGIN_QCALL;

_ASSERTE(handle != NULL);
_ASSERTE(handle != INVALID_HANDLE_VALUE);

result = PAL_WaitForSingleObjectPrioritized(handle, timeoutMs);

END_QCALL;
return (INT32)result;
}
#endif // TARGET_UNIX
17 changes: 7 additions & 10 deletions src/coreclr/vm/comwaithandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,12 @@
#ifndef _COM_WAITABLE_HANDLE_H
#define _COM_WAITABLE_HANDLE_H

extern "C" INT32 QCALLTYPE WaitHandle_WaitOneCore(HANDLE handle, INT32 timeout, BOOL useTrivialWaits);
extern "C" INT32 QCALLTYPE WaitHandle_WaitMultipleIgnoringSyncContext(HANDLE *handleArray, INT32 numHandles, BOOL waitForAll, INT32 timeout);
extern "C" INT32 QCALLTYPE WaitHandle_SignalAndWait(HANDLE waitHandleSignal, HANDLE waitHandleWait, INT32 timeout);

class WaitHandleNative
{
public:
static FCDECL3(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout, FC_BOOL_ARG useTrivialWaits);
static FCDECL4(INT32, CorWaitMultipleNative, HANDLE *handleArray, INT32 numHandles, FC_BOOL_ARG waitForAll, INT32 timeout);
static FCDECL3(INT32, CorSignalAndWaitOneNative, HANDLE waitHandleSignalUNSAFE, HANDLE waitHandleWaitUNSAFE, INT32 timeout);
};
#ifdef TARGET_UNIX
extern "C" INT32 QCALLTYPE WaitHandle_CorWaitOnePrioritizedNative(HANDLE handle, INT32 timeoutMs);
#endif
#endif
extern "C" INT32 QCALLTYPE WaitHandle_WaitOnePrioritized(HANDLE handle, INT32 timeoutMs);
#endif // TARGET_UNIX

#endif // _COM_WAITABLE_HANDLE_H
7 changes: 0 additions & 7 deletions src/coreclr/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,6 @@ FCFuncStart(gThreadPoolFuncs)
FCFuncElement("GetNextConfigUInt32Value", ThreadPoolNative::GetNextConfigUInt32Value)
FCFuncEnd()

FCFuncStart(gWaitHandleFuncs)
FCFuncElement("WaitOneCore", WaitHandleNative::CorWaitOneNative)
FCFuncElement("WaitMultipleIgnoringSyncContext", WaitHandleNative::CorWaitMultipleNative)
FCFuncElement("SignalAndWaitNative", WaitHandleNative::CorSignalAndWaitOneNative)
FCFuncEnd()

FCFuncStart(gCastHelpers)
FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup)
FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup)
Expand Down Expand Up @@ -493,7 +487,6 @@ FCClassElement("Thread", "System.Threading", gThreadFuncs)
FCClassElement("ThreadPool", "System.Threading", gThreadPoolFuncs)
FCClassElement("Type", "System", gSystem_Type)
FCClassElement("TypedReference", "System", gTypedReferenceFuncs)
FCClassElement("WaitHandle", "System.Threading", gWaitHandleFuncs)

#undef FCFuncElement
#undef FCFuncElementSig
Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/vm/qcallentrypoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,12 @@ static const Entry s_QCall[] =
#ifdef FEATURE_COMINTEROP
DllImportEntry(ThreadNative_DisableComObjectEagerCleanup)
#endif // FEATURE_COMINTEROP
DllImportEntry(WaitHandle_WaitOneCore)
DllImportEntry(WaitHandle_WaitMultipleIgnoringSyncContext)
DllImportEntry(WaitHandle_SignalAndWait)
#ifdef TARGET_UNIX
DllImportEntry(WaitHandle_CorWaitOnePrioritizedNative)
#endif
DllImportEntry(WaitHandle_WaitOnePrioritized)
#endif // TARGET_UNIX
DllImportEntry(ClrConfig_GetConfigBoolValue)
DllImportEntry(Buffer_Clear)
DllImportEntry(Buffer_MemMove)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3116,14 +3116,14 @@ DWORD MsgWaitHelper(int numWaiters, HANDLE* phEvent, BOOL bWaitAll, DWORD millis
// want true WAIT_ALL, we need to fire up a different thread in the MTA and wait
// on its result. This isn't implemented yet.
//
// A change was added to WaitHandleNative::CorWaitMultipleNative to disable WaitAll
// A change was added to WaitHandle_WaitMultipleIgnoringSyncContext to disable WaitAll
// in an STA with more than one handle.
if (bWaitAll)
{
if (numWaiters == 1)
bWaitAll = FALSE;

// The check that's supposed to prevent this condition from occurring, in WaitHandleNative::CorWaitMultipleNative,
// The check that's supposed to prevent this condition from occurring, in WaitHandle_WaitMultipleIgnoringSyncContext,
// is unfortunately behind FEATURE_COMINTEROP instead of FEATURE_COMINTEROP_APARTMENT_SUPPORT.
// So on CoreCLR (where FEATURE_COMINTEROP is not currently defined) we can actually reach this point.
// We can't fix this, because it's a breaking change, so we just won't assert here.
Expand Down

0 comments on commit 10f6c4c

Please sign in to comment.