3

The application I'm developing requires that some data is obtained through different channels. As this is a network process, I have decided to use async programming.

I have designed the following interface to represent all the possible channels:

public interface IMyDataChannel
{
    Task<MyData> GetMyDataAsync(/* parameters not relevant to the question */);
}

I need to obtain this data with different parameters, and from different channels sometimes, so ideally I should throw all the requests in paralell for the best performance.

The problem is that one of the channels is very unstable, and if I throw several requests in parallel, or even sequentially, it overloads and returns unexpected results. I need to make sure that this channel does not support parallel requests and waits for some time after each request, to avoid overloading it.

I think this logic belongs in the channel implementation, because callers should not be aware if the underlaying channel is stable or not. Instead, they should fetch the data from the channels in an unified way.

First, I tought I could use a static lock in the implementation of the unstable channel, and make sure that the process sleeps for some time before releasing the lock. The implementation would be like this:

public class UnstableMyDataChannel : IMyDataChannel
{
    private const int stabilityDelay = 1000;
    private static readonly object stabilityLock = new object();

    public async Task<MyData> GetMyDataAsync(/* parameters not relevant to the question */)
    {
        lock(stabilityLock)
        {
            //Throws a compiler error because you can't "await" inside a "lock"
            MyData data = await PrivateMethodsForRetrievingDataAsync(/* parameters not relevant to the question */);

            //Throws a compiler error because you can't "await" inside a "lock"
            await Task.Delay(stabilityDelay);
            return data;
        }
    }
}

However, you can't await anything inside a lock statement. Even if you could, this design doesn't completely convince me. Can anyone provide a better (and possible) solution?

  • 1
    Is the part where the data is obtained protected against running without termination? Otherwise you risk to get a race condition. – Doc Brown Feb 19 '18 at 13:57
  • What do you mean? Are you talking about the possibility of exceptions or something else? – Daniel García Rubio Feb 19 '18 at 14:19
  • No, this is not about exceptions. I thought about a situation where the lock is obtained but never released, because the code section you marked as //This is where the data is obtained might stick in an endless loop. If that channel is that "unstable" as you say, who knows what can happen there? – Doc Brown Feb 19 '18 at 15:36
  • Does this compile? I get "CS1996 Cannot await in the body of a lock statement" – Erik Eidt Feb 19 '18 at 15:40
  • @ErikEidt That's correct, it doesn't even compile. I didn't know that limitation. I will update my question. – Daniel García Rubio Feb 19 '18 at 15:46
5

First, I would create a decorator / wrapper that synchronizes calls, instead of putting it into implementation.

And I would implement it using queue instead of using a lock. TaskQueue from this other question could be used.

My quickly hacked together testing code :

public interface IMyDataChannel
{
    Task<string> GetMyDataAsync(int parameter);
}

public class MockDataChannel : IMyDataChannel
{
    public async Task<string> GetMyDataAsync(int parameter)
    {
        Console.WriteLine(DateTime.Now.ToLongTimeString() +  " starting " + parameter);

        await Task.Delay(parameter * 300);

        Console.WriteLine(DateTime.Now.ToLongTimeString() + " ending " + parameter);

        return parameter.ToString();
    }
}

public class DataChannelSynchronized : IMyDataChannel
{
    private readonly IMyDataChannel inner;
    private readonly TaskQueue queue = new TaskQueue();

    public DataChannelSynchronized(IMyDataChannel inner)
    {
        this.inner = inner;
    }

    public Task<string> GetMyDataAsync(int parameter)
    {
        return queue.Enqueue(() => Delay(inner.GetMyDataAsync(parameter), 1000));
    }

    public static async Task<T> Delay<T>(Task<T> task, int delay)
    {
        var res = await task;

        await Task.Delay(delay);

        return res;
    }
}

public class Program
{
    public static void Main(string[] args)
    {
        var channel = new DataChannelSynchronized(new MockDataChannel());

        Task.WhenAll(channel.GetMyDataAsync(1),
        channel.GetMyDataAsync(3),
        channel.GetMyDataAsync(4)).Wait();


    }
}
  • Well, this seems perfectly adequate to what i'm trying to achieve. I guess I could implement something like this for the channel(s) that do not support parallel requests, and simply return regular tasks for the rest of the channels. It feels elegant as well, as it actually does what it should do (enqueue tasks that shouldn't be parallel) instad of trying to "hack" the language. – Daniel García Rubio Feb 19 '18 at 17:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.