From: Jan Kotas Date: Mon, 13 Jan 2020 02:58:25 +0000 (-0800) Subject: Throw NotSupportedException for WaitHandle.WaitAny with 64 handle on STA threads... X-Git-Tag: submit/tizen/20210909.063632~10453 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d74ea2f0138a084392fab5bec67ea78478791f85;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Throw NotSupportedException for WaitHandle.WaitAny with 64 handle on STA threads (#1647) This change is technically a breaking change, but it is better to throw than silently do the wrong thing. Fixes #1646 --- diff --git a/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx b/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx index add5d7a..e163e2f 100644 --- a/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx +++ b/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx @@ -2896,6 +2896,9 @@ The number of WaitHandles must be less than or equal to 64. + + The number of WaitHandles on a STA thread must be less than or equal to 63. + Memory stream is not expandable. diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp index 9affaf7..4624156 100644 --- a/src/coreclr/src/vm/threads.cpp +++ b/src/coreclr/src/vm/threads.cpp @@ -3203,22 +3203,11 @@ DWORD Thread::DoAppropriateWait(AppropriateWaitFunc func, void *args, //-------------------------------------------------------------------- DWORD MsgWaitHelper(int numWaiters, HANDLE* phEvent, BOOL bWaitAll, DWORD millis, BOOL bAlertable) { - STATIC_CONTRACT_THROWS; - // The true contract for GC trigger should be the following. But this puts a very strong restriction - // on contract for functions that call EnablePreemptiveGC. - //if (GetThread() && !ThreadStore::HoldingThreadStore(GetThread())) {GC_TRIGGERS;} else {GC_NOTRIGGER;} - STATIC_CONTRACT_GC_TRIGGERS; + STANDARD_VM_CONTRACT; DWORD flags = 0; DWORD dwReturn=WAIT_ABANDONED; - Thread* pThread = GetThread(); - // If pThread is NULL, we'd better shut down. - if (pThread == NULL) - _ASSERTE (g_fEEShutDown); - - DWORD lastError = 0; - // If we're going to pump, we cannot use WAIT_ALL. That's because the wait would // only be satisfied if a message arrives while the handles are signalled. If we // want true WAIT_ALL, we need to fire up a different thread in the MTA and wait @@ -3244,8 +3233,12 @@ DWORD MsgWaitHelper(int numWaiters, HANDLE* phEvent, BOOL bWaitAll, DWORD millis if (bAlertable) flags |= COWAIT_ALERTABLE; - HRESULT hr = S_OK; - hr = CoWaitForMultipleHandles(flags, millis, numWaiters, phEvent, &dwReturn); + // CoWaitForMultipleHandles does not support more than 63 handles. It returns RPC_S_CALLPENDING for more than 63 handles + // that is impossible to differentiate from timeout. + if (numWaiters > 63) + COMPlusThrow(kNotSupportedException, W("NotSupported_MaxWaitHandles_STA")); + + HRESULT hr = CoWaitForMultipleHandles(flags, millis, numWaiters, phEvent, &dwReturn); if (hr == RPC_S_CALLPENDING) { @@ -3259,13 +3252,9 @@ DWORD MsgWaitHelper(int numWaiters, HANDLE* phEvent, BOOL bWaitAll, DWORD millis dwReturn = WAIT_FAILED; } else -{ + { dwReturn += WAIT_OBJECT_0; // success -- bias back - } - - lastError = ::GetLastError(); - - ::SetLastError(lastError); + } return dwReturn; } @@ -3277,11 +3266,7 @@ DWORD MsgWaitHelper(int numWaiters, HANDLE* phEvent, BOOL bWaitAll, DWORD millis DWORD Thread::DoAppropriateAptStateWait(int numWaiters, HANDLE* pHandles, BOOL bWaitAll, DWORD timeout, WaitMode mode) { - CONTRACTL { - THROWS; - GC_TRIGGERS; - } - CONTRACTL_END; + STANDARD_VM_CONTRACT; BOOL alertable = (mode & WaitMode_Alertable) != 0; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs index 3dfe5b6..6909892 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs @@ -261,15 +261,6 @@ namespace System.Threading { if (waitHandles.Length == 0) { - // - // Some history: in CLR 1.0 and 1.1, we threw ArgumentException in this case, which was correct. - // Somehow, in 2.0, this became ArgumentNullException. This was not fixed until Silverlight 2, - // which went back to ArgumentException. - // - // Now we're in a bit of a bind. Backward-compatibility requires us to keep throwing ArgumentException - // in CoreCLR, and ArgumentNullException in the desktop CLR. This is ugly, but so is breaking - // user code. - // throw new ArgumentException(SR.Argument_EmptyWaithandleArray, nameof(waitHandles)); } if (waitHandles.Length > MaxWaitHandles) diff --git a/src/libraries/System.Runtime/tests/System/Threading/WaitHandleTests.cs b/src/libraries/System.Runtime/tests/System/Threading/WaitHandleTests.cs index debcaff..1432b7a 100644 --- a/src/libraries/System.Runtime/tests/System/Threading/WaitHandleTests.cs +++ b/src/libraries/System.Runtime/tests/System/Threading/WaitHandleTests.cs @@ -78,14 +78,39 @@ namespace System.Threading.Tests Assert.Equal(0, WaitHandle.WaitAny(wh)); } + static ManualResetEvent[] CreateManualResetEvents(int length) + { + var handles = new ManualResetEvent[length]; + for (int i = 0; i < handles.Length; i++) + handles[i] = new ManualResetEvent(true); + return handles; + } + + [Fact] + public static void WaitAny_MaxHandles() + { + Assert.Equal(0, WaitHandle.WaitAny(CreateManualResetEvents(64))); + Assert.Throws(() => WaitHandle.WaitAny(CreateManualResetEvents(65))); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [PlatformSpecific(TestPlatforms.Windows)] + public static void WaitAny_MaxHandles_STA() + { + Thread t = new Thread(() => + { + Assert.Equal(0, WaitHandle.WaitAny(CreateManualResetEvents(63))); + Assert.Throws(() => WaitHandle.WaitAny(CreateManualResetEvents(64))); + }); + t.SetApartmentState(ApartmentState.STA); + t.Start(); + t.Join(); + } + [Fact] public static void WaitAll() { - var handles = new ManualResetEvent[] { - new ManualResetEvent(true), - new ManualResetEvent(true), - new ManualResetEvent(true) - }; + ManualResetEvent[] handles = CreateManualResetEvents(3); Assert.True(WaitHandle.WaitAll(handles)); Assert.True(WaitHandle.WaitAll(handles, 1));