Is SemaphoreSlim needed when Channel's SingleReader is set to true

Question

When sending data fast enough, InvalidOperationException is being thrown: 'There is already one outstanding 'SendAsync' call for this WebSocket instance. ClientWebSocket.ReceiveAsync and ClientWebSocket.SendAsync can be called simultaneously, but at most one outstanding operation for each of them is allowed at the same time.

The first example uses SemaphoreSlim which allows only one message at a time that prevents that issue from happening.

The question is do I need to do the same SemaphoreSlim workaround in the second example since SingleReader = true is specified in the channel options? It should basically be the same, but I would like someone to confirm it, so there are no surprises.

Without System.Threading.Channels

private readonly SemaphoreSlim sendAsyncSemaphore = new SemaphoreSlim(1);

public async Task<bool> SendMessageAsync(ReadOnlyMemory<byte> message)
{
    if (_webSocket.State != WebSocketState.Open)
    {
        return false;
    }
    
    // Only one message can be sent at a time. Wait until a message can be sent.
    await sendAsyncSemaphore.WaitAsync().ConfigureAwait(false);
    
    // We can now send a message. Our thread has entered sendAsyncSemaphore, so we should be the one to release it.
    try
    {
        await _Client.SendAsync(message, WebSocketMessageType.Text, true, connectionCancellation.Token).ConfigureAwait(true);
    }
    catch (TaskCanceledException)
    {
        sendAsyncSemaphore.Release();
        return false;
    }
    
    // Our thread can now release the sendAsyncSemaphore so another message can be sent.
    sendAsyncSemaphore.Release();
    return true;
}

public void Dispose()
{
    sendAsyncSemaphore.Dispose();
}

With System.Threading.Channels

public class WebSocketClient
{
    private readonly WebSocket _webSocket;
    private readonly Channel<string> _input;
    private readonly Channel<string> _output;

    public WebSocketClient(WebSocket webSocket)
    {
        _webSocket = webSocket;

        _input = Channel.CreateUnbounded<string>(new UnboundedChannelOptions
        {
            SingleWriter = true
        });

        _output = Channel.CreateUnbounded<string>(new UnboundedChannelOptions
        {
            SingleReader = true
        });
    }

    private async Task SendLoopAsync(CancellationToken cancellationToken)
    {
        await foreach (var message in _output.Reader.ReadAllAsync(cancellationToken))
        {
            if (_webSocket.State != WebSocketState.Open)
            {
                return;
            }

            var bytesToSend = new ArraySegment<byte>(Encoding.UTF8.GetBytes(message));
            await _webSocket.SendAsync(bytesToSend, WebSocketMessageType.Text, true, cancellationToken).ConfigureAwait(false);
        }
    }
}

`SingleReader` is an optimization setting telling the Channel there's only one possible reader and it should disable its synchronization code. *Don't* use it if there's a chance more than two reader will be active. In your code, assuming nobody else uses that channel, there won't be any other reader and no lock is necessary — Panagiotis Kanavos, Apr 11 '22 at 16:35

HasaniH · Accepted Answer · 2022-04-07T13:57:03.107

2

Short answer yes! You still need something to limit the number of outstanding read calls when SingleReader = true. The documentation clearly states, you set that property to true 'if readers to the channel guarantee that there will only ever be at most one read operation at a time'. So the property does not provide limitation on the senders.

Using a semaphore or any other synchronization primitive is not a work around in my opinion. If you take a look at the code, you'll see that setting the SingleReader option gets you a SingleConsumerUnboundedChannel. If you don't limit the readers yourself you'll end up unintentionally cancelling read operations.

edited Apr 07 '22 at 13:57

answered Apr 07 '22 at 13:24

HasaniH

8,232
6
41
59

Thank you for your answer! Simplified, SingleReader set to true is enough to do the job and SemaphoreSlim in the second example is not required right? – nop Apr 07 '22 at 13:33
Depends on how you call `SendLoopAsync` in your second example. I think if you make multiple calls to that method without synchronization, you'll get unexpected behavior – HasaniH Apr 07 '22 at 14:04
I'm calling it like `_ = Task.Factory.StartNew(_ => SendLoopAsync(cancellationToken), TaskCreationOptions.LongRunning, cancellationToken);` and `public ValueTask SendAsync(string message) { return _output.Writer.WriteAsync(message); }` – nop Apr 07 '22 at 16:50
I think I understand what you meant. If there are x5 `SendAsync()` calls at the same time and the first one is being processed, it's up to us what's going to happen to the other x4 items, i.e. based on `BoundedChannelFullMode`. If it's set to `DropOldest` = it drops the awaiting calls during the execution of the first one. I think by default it is set to `Wait` and it's going to wait and process them all. Am I wrong? – nop Apr 07 '22 at 16:56
a channel is essentially a queue which regulates how much items you can have in a queue or who can read from a queue or when you can read off a queue in an asynchronous manner, so only read when we have items on there – nop Apr 07 '22 at 17:17
1

@nop if you want to understand how Channels are meant to be used, check Go's channels. A Channel isn't a general purpose async queue, although it can be used like that with some extra effort. It's an asynchronous producer/consumer queue, where the producer and consumer are separate and only communicate through that channel – Panagiotis Kanavos Apr 11 '22 at 16:38

D-Shih · Answer 2 · 2022-04-07T13:36:34.713

If you set SingleReader = true you unnecessary use SemaphoreSlim because that only allows true readers from the channel to guarantee that there will only ever be at most one read operation at a time.

true readers from the channel guarantee that there will only ever be at most one read operation at a time; if no such constraint is guaranteed.false

BTW you are only defining initialCount but didn't limit the maxCount which means you will unlimited signal count, here is the SemaphoreSlim source code, NO_MAXIMUM is a const value int.MaxValue.

// No maximum constant
private const int NO_MAXIMUM = int.MaxValue;


public SemaphoreSlim(int initialCount)
    : this(initialCount, NO_MAXIMUM)
{
}

you can try to use another construct method which provides 2 parameters, the first one is initialCount second one is setting maxCount

private readonly SemaphoreSlim sendAsyncSemaphore = new SemaphoreSlim(1,1);

Is SemaphoreSlim needed when Channel's SingleReader is set to true

Without System.Threading.Channels

With System.Threading.Channels

2 Answers2