In our code we have a threading abstraction similar to a Reactive EventLoopScheduler where we can post work to a single background thread. Our implementation has a facility for detecting if the call to post is already on thread and will execute the work synchronously. We also have an implementation of SynchronizationContext that will post to this thread. This is so continuation of async/awaits within the posted work will complete on the dedicated thread.

However we encountered an issue when using SemaphoreSlim. It seems it is possible for code to re-enter the Release call and corrupt the state of the semaphore when the SynchronizationContext.Post call executes the callback synchronously. See below code for a reproduction case.

    var semaphore = new SemaphoreSlim(1, 1);
    var scheduler = new EventLoopScheduler(ts => new Thread(ts) { Name = "MyThread", IsBackground = true });
    scheduler.Schedule (() => SynchronizationContext.SetSynchronizationContext(new MySynchronizationContext(scheduler)));

    var tasks = new List<Task>();
    for (int i = 0; i < 10; i++)
    {
        tasks.Add(scheduler.ScheduleTask(DoSomething, i));
    await Task.Delay(TimeSpan.FromMilliseconds(100));
    }
    Task.WaitAll(tasks.ToArray());

    async Task DoSomething(int i)
    {

        Debug.WriteLine($"[{Thread.CurrentThread.Name}] Waiting {i}");
        await semaphore.WaitAsync();
    try
    {
        Debug.WriteLine($"[{Thread.CurrentThread.Name}] Entered {i}");
        if (i != 0)
        {
            return;
        }

        await Task.Delay(TimeSpan.FromMilliseconds(400));
    }
    finally
    {
        Debug.WriteLine($"[{Thread.CurrentThread.Name}] Release Start {i}");
        semaphore.Release();
        Debug.WriteLine($"[{Thread.CurrentThread.Name}] Release End {i}");
    }
    }

    public static class SchedulerExtensions
    {

    public static Task ScheduleTask(this IScheduler scheduler, Func<int,Task> task, int i)
    {
        var tcs = new TaskCompletionSource<bool>();
        scheduler.Schedule(async () =>
        {
            try
            {
                await task(i);
                tcs.SetResult(true);
            }
            catch (Exception e)
            {
                tcs.SetException(e);
            }
        });
        return tcs.Task;
    }
    }

    public class MySynchronizationContext : SynchronizationContext
    {
    IScheduler _s;
    public MySynchronizationContext(IScheduler s)
    {
        _s = s;
    }

    public override void Post(SendOrPostCallback d, object state)
    {
        // Comment this if statement out to prevent semaphore re-entrance.
        if(Thread.CurrentThread.Name == "MyThread") {
            d.Invoke(state);
            return;
        }
        
        _s.Schedule (() => d.Invoke(state));
    }
    }

When MySynchronization allows synchronous Post the output is:

synchronous Post

Note the re-entrance of tasks 1, 2 and 3 before 0 is complete. This corrupts the semaphore count and task there after wait forever.

When we make Post asynchronous the output is:

asynchronous Post

Note how end release occurs before the next task enters the semaphore.

It seems a similar issue has come up before here.

With a solution submitted [here].(https://github.com/dotnet/corefx/commit/ecba811b1438517ac90a957e2cfe4cef64a13861)

But this solution seems to assume the default SynchronizationContext, which asynchronously posts to the ThreadPool.

Is it expected that Post never executes the callback synchronously?

This MSDN article documents that the ASP context posts synchronously. However it seems that ASP dropped the SynchronizationContext in Core. Also this same article states doing so “may cause unexpected re-entrancy issues.”

Based on the above I am tending to believe that Post should always be asynchronous, but the fact that MS has an implementation counter to that (albeit deprecated) has us wondering if we assume too much. We are exploring changing our implementation of SynchronizationContext but its not the type of change we can make lightly.