Skip to content

Commit

Permalink
Fix bug 60568: SynchronizationContext.Wait not implemented or invoked…
Browse files Browse the repository at this point in the history
… by runtime (#6062)

* Checkpoint: Implement SynchronizationContext.Wait

* Add missing check for null synchronization context

* Update bug 60568 fix to cover WaitAll and add test cases

* Add missing file
  • Loading branch information
kg authored and luhenry committed Dec 14, 2017
1 parent 5e90816 commit 252d61d
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 32 deletions.
1 change: 1 addition & 0 deletions mcs/class/System/System_test.dll.sources
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ System.Threading/SemaphoreFullExceptionCas.cs
System.Threading/SemaphoreTest.cs
System.Threading/BarrierTest.cs
System.Threading/ThreadExceptionEventArgsCas.cs
System.Threading/WaitHandleTests.cs
System.Timers/TimersDescriptionAttributeCas.cs
System.Timers/ElapsedEventArgsCas.cs
System.Timers/TimerCas.cs
Expand Down
65 changes: 65 additions & 0 deletions mcs/class/System/Test/System.Threading/WaitHandleTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using NUnit.Framework;

using System;
using System.Security.AccessControl;
using System.Threading;

namespace MonoTests.System.Threading {
[TestFixture]
public class WaitHandleTests {
SynchronizationContext OriginalContext;
TestSynchronizationContext TestContext;

[SetUp]
public void SetUp ()
{
OriginalContext = SynchronizationContext.Current;
TestContext = new TestSynchronizationContext();
TestContext.SetWaitNotificationRequired();
SynchronizationContext.SetSynchronizationContext(TestContext);
}

[TearDown]
public void TearDown ()
{
SynchronizationContext.SetSynchronizationContext(OriginalContext);
}

[Test]
public void WaitHandle_WaitOne_SynchronizationContext ()
{
var e = new ManualResetEvent(false);
TestContext.WaitAction = () => e.Set();
Assert.IsTrue(e.WaitOne(0));
}

[Test]
public void WaitHandle_WaitAll_SynchronizationContext ()
{
var e1 = new ManualResetEvent(false);
var e2 = new ManualResetEvent(false);
TestContext.WaitAction = () => {
e1.Set();
e2.Set();
};
Assert.IsTrue(WaitHandle.WaitAll(new[] { e1, e2 }, 0));
}
}

class TestSynchronizationContext : SynchronizationContext
{
public Action WaitAction { get; set; }

public new void SetWaitNotificationRequired ()
{
base.SetWaitNotificationRequired();
}

public override int Wait (IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout)
{
WaitAction?.Invoke();
return base.Wait(waitHandles, waitAll, millisecondsTimeout);
}
}
}

99 changes: 68 additions & 31 deletions mcs/class/corlib/System.Threading/WaitHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,63 @@ public abstract partial class WaitHandle

internal const int MaxWaitHandles = 64;

// We rely on the reference source implementation of WaitHandle, and it delegates to a function named
// WaitOneNative to perform the actual operation of waiting on a handle.
// This native operation actually has to call back into managed code and invoke .Wait
// on the current SynchronizationContext. As such, our implementation of this "native" method
// is actually managed code, and the real native icall being used is Wait_internal.
static int WaitOneNative (SafeHandle waitableSafeHandle, uint millisecondsTimeout, bool hasThreadAffinity, bool exitContext)
{
bool release = false;
var context = SynchronizationContext.Current;
try {
waitableSafeHandle.DangerousAddRef (ref release);

#if !DISABLE_REMOTING
if (exitContext)
SynchronizationAttribute.ExitContext ();
#endif

// HACK: Documentation (and public posts by experts like Joe Duffy) suggests that
// users must first call SetWaitNotificationRequired to flag that a given synchronization
// context overrides .Wait. Because invoking the Wait method is somewhat expensive, we use
// the notification-required flag to determine whether or not we should invoke the managed
// wait method.
// Another option would be to check whether this context uses the default Wait implementation,
// but I don't know of a cheap way to do this that handles derived types correctly.
// If the thread does not have a synchronization context set at all, we can safely just
// jump directly to invoking Wait_internal.
if ((context != null) && context.IsWaitNotificationRequired ()) {
return context.Wait (
new IntPtr[] { waitableSafeHandle.DangerousGetHandle () },
false,
(int)millisecondsTimeout
);
} else {
unsafe {
IntPtr handle = waitableSafeHandle.DangerousGetHandle ();
return Wait_internal (&handle, 1, false, (int)millisecondsTimeout);
}
}
} finally {
if (release)
waitableSafeHandle.DangerousRelease ();

#if !DISABLE_REMOTING
if (exitContext)
SynchronizationAttribute.EnterContext ();
#endif
}

}

static int WaitMultiple(WaitHandle[] waitHandles, int millisecondsTimeout, bool exitContext, bool WaitAll)
{
if (waitHandles.Length > MaxWaitHandles)
return WAIT_FAILED;

int release_last = -1;
var context = SynchronizationContext.Current;

try {
#if !DISABLE_REMOTING
Expand All @@ -70,45 +121,31 @@ static int WaitMultiple(WaitHandle[] waitHandles, int millisecondsTimeout, bool
}
}

unsafe {
IntPtr* handles = stackalloc IntPtr[waitHandles.Length];

if ((context != null) && context.IsWaitNotificationRequired ()) {
IntPtr[] handles = new IntPtr[waitHandles.Length];
for (int i = 0; i < waitHandles.Length; ++i)
handles[i] = waitHandles[i].SafeWaitHandle.DangerousGetHandle ();

return Wait_internal(handles, waitHandles.Length, WaitAll, millisecondsTimeout);
return context.Wait (
handles,
false,
(int)millisecondsTimeout
);
} else {
unsafe {
IntPtr* handles = stackalloc IntPtr[waitHandles.Length];

for (int i = 0; i < waitHandles.Length; ++i)
handles[i] = waitHandles[i].SafeWaitHandle.DangerousGetHandle ();

return Wait_internal (handles, waitHandles.Length, WaitAll, millisecondsTimeout);
}
}
} finally {
for (int i = release_last; i >= 0; --i) {
waitHandles [i].SafeWaitHandle.DangerousRelease ();
}

#if !DISABLE_REMOTING
if (exitContext)
SynchronizationAttribute.EnterContext ();
#endif
}
}

static int WaitOneNative (SafeHandle waitableSafeHandle, uint millisecondsTimeout, bool hasThreadAffinity, bool exitContext)
{
bool release = false;
try {
#if !DISABLE_REMOTING
if (exitContext)
SynchronizationAttribute.ExitContext ();
#endif

waitableSafeHandle.DangerousAddRef (ref release);

unsafe {
IntPtr handle = waitableSafeHandle.DangerousGetHandle();
return Wait_internal(&handle, 1, false, (int)millisecondsTimeout);
}
} finally {
if (release)
waitableSafeHandle.DangerousRelease ();

#if !DISABLE_REMOTING
if (exitContext)
SynchronizationAttribute.EnterContext ();
Expand All @@ -117,7 +154,7 @@ static int WaitOneNative (SafeHandle waitableSafeHandle, uint millisecondsTimeou
}

[MethodImplAttribute(MethodImplOptions.InternalCall)]
unsafe static extern int Wait_internal(IntPtr* handles, int numHandles, bool waitAll, int ms);
internal unsafe static extern int Wait_internal(IntPtr* handles, int numHandles, bool waitAll, int ms);

static int SignalAndWaitOne (SafeWaitHandle waitHandleToSignal,SafeWaitHandle waitHandleToWaitOn, int millisecondsTimeout, bool hasThreadAffinity, bool exitContext)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ public virtual int Wait(IntPtr[] waitHandles, bool waitAll, int millisecondsTime
#if MONO
protected static int WaitHelper(IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout)
{
throw new NotImplementedException ();
unsafe {
fixed (IntPtr * pWaitHandles = waitHandles) {
return System.Threading.WaitHandle.Wait_internal (pWaitHandles, waitHandles.Length, waitAll, millisecondsTimeout);
}
}
}
#else
[MethodImplAttribute(MethodImplOptions.InternalCall)]
Expand Down

0 comments on commit 252d61d

Please sign in to comment.