0

My app has a concept like global resources. There are collections of objects contained which each part in my solution needs. Some of them are loaded at startup and other at first access.

My question goes to the second case (the lazy loaded collections). Below you see a test project:

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("Wellcome!");

        var globalResources = new GlobalResources();

        var toolClassA = new MyToolClass(globalResources);
        var toolClassB = new MyToolClass(globalResources);

        // This two "Init" calls can not be awaited in real project.
        // They could be invoked by two different buttons
        _ = toolClassA.InitAsync();
        _ = toolClassB.InitAsync();

        Console.WriteLine("Finish");
        Console.ReadLine();
    }
}

public class MyToolClass
{
    private readonly GlobalResources _globalResources;

    public MyToolClass(GlobalResources globalResources)
    {
        _globalResources = globalResources ?? throw new ArgumentNullException(
            nameof(globalResources));
    }

    public async Task InitAsync()
    {
        await _globalResources.LoadAsync();
        Console.WriteLine("awaited");
    }
}

public class GlobalResources
{
    public bool _isLoaded;
    public readonly object _loadLock = new object();

    private string _externalQueryResult;

    private Task _runningLoadTask;

    public async Task LoadAsync()
    {
        if (_isLoaded)
        {
            return;
        }

        lock (_loadLock)
        {
            if (_isLoaded)
            {
                return;
            }
        }

        // A: Current code:
        {
            _externalQueryResult = await LongRunningQuery();
        }

        // B: Wanting something like this:
        {
            // If no task is already running then start one.
            if (_runningLoadTask == null)
            {
                _runningLoadTask = LongRunningQuery();
                await _runningLoadTask.Start(); // How await it?
            }
            // If a task ist already running then no db call should happen.
            // Instead the currently running task should be awaited.
            else
            {
                await Task.WaitAll(_runningLoadTask); // ???
                return;
            }
        }

        lock (_loadLock)
        {
            _isLoaded = true;
        }
    }

    public async Task<string> LongRunningQuery()
    {
        // This method should only be called once.
        Console.WriteLine("Loading data from Database (
            this line should only be written once on console window).");

        await Task.Delay(5000);
        return "123";
    }
}

The problem I have is in class GlobalResources method LoadAsync. There you can find a comment "A" which represents the way I code currently. That way is a problem because if a second load request happens then I have two database calls.

My solution you can find in section "B". I want to store the first task of the first request in a field. If a second call comes in it should await the stored Task which prevent it from call the db a second time.

My problem is I don't know how to code it. I think the async await pattern does not work if I work with Tasks manually. Or have you any idea?

Alois
  • 361
  • 2
  • 18
  • 2
    You're looking for... `await`. The job of `await` is to wait for an *already started* task to complete. The job of starting that task falls to something else. `LongRunningQuery`, in this case, already returns you a started task. You need only `await` it. It's `await`, not `astart`. – Damien_The_Unbeliever Jan 19 '21 at 15:46
  • 3
    That's absolutely fine, and a common pattern. You can `await` a Task multiple times. If the Task isn't yet complete, the awai will wait. If it is, the await returns synchronously. (That isn't the case with `ValueTask` however, so watch out) – canton7 Jan 19 '21 at 15:46
  • 4
    (You do need to watch out for your use of `lock` however - if you're going to use locks, it should ideally cover the creation of the task too, otherwise you still have a race) – Damien_The_Unbeliever Jan 19 '21 at 15:55
  • @Damien_The_Unbeliever: Thanks, the first async task seems to work if I delete the 'Start' method. – Alois Jan 19 '21 at 15:55
  • @canton7: Do you know how I have to write the "WaitAll" so the await-pattern ist working? – Alois Jan 19 '21 at 15:56
  • @Alois I'm not sure what you're trying to ask in your comment, I'm afraid – canton7 Jan 19 '21 at 15:59
  • You should consider using `Lazy` or `Lazy>` if you want to load things lazily. – JonasH Jan 19 '21 at 16:00
  • 1
    @Alois - you don't need `WaitAll`. `_runningLoadTask` is an already started (and possibly finished) task which, again, you can just `await`. – Damien_The_Unbeliever Jan 19 '21 at 16:00
  • @Damien_The_Unbeliever: That was it. Thanks! Addittionaly I found that I has to call `Task.WhenAll` instead of `Task.WaitAll`. Thanks for your help :-D – Alois Jan 19 '21 at 16:09
  • Related: [Enforce an async method to be called once](https://stackoverflow.com/questions/28340177/enforce-an-async-method-to-be-called-once) – Theodor Zoulias Jan 19 '21 at 16:26
  • Alois I would suggest to remove the solution you posted inside the question, and post it instead as a separate answer. Bundling a question and an answer in a single post is against the StackOverflow etiquette. On the other hand [answering your own question](https://stackoverflow.com/help/self-answer) is not only perfectly OK, but actually encouraged. – Theodor Zoulias Jan 19 '21 at 16:35
  • @Theodor Zoulias: Thanks for the info – Alois Jan 20 '21 at 06:16

2 Answers2

3

First of all, it is OK to await the same task multiple times and it not gonna hurt your database. So, at least, you could cache such task (you're locking anyways, so it's safe) and later reuse it. That lets you to minimize locking time btw.

Secondly, since your resources are global by definition, it seems reasonable to precompute long-running query. Which means you run appropriate task right in the constructor (without awaiting it) and no locking is needed, not at all.

public MyConstructor()
{
    _myCachedTask = LongRunningTask(); // don't await it; hopefully, task is now running somewhere on the thread pool
}

...

public Task LongRunningTask() => _myCachedTask; // it's OK to await it multiple times, thread safe as well
Zazaeil
  • 3,900
  • 2
  • 14
  • 31
  • Good to hear that the idea of awaiting a task multiple times if fine. The preloaded collections I will load in constructor but the lazy ones at first request. Have you any idea how to combine Task.WaitAll with the async await pattern? – Alois Jan 19 '21 at 16:00
  • 1
    `await Task.WhenAll(listOfTaks)`. Not sure what you mean. – Zazaeil Jan 19 '21 at 16:17
  • 1
    That method I was looking for. I didn't realize it exists. Thanks – Alois Jan 19 '21 at 16:18
1

Based on the comments I was possible to change the B-block so it is compileable. You can find the code below:

As damien-the-unbeliever said in a comment, the posted solution is not thread safe! There are missing some locks.

// B: Wanting something like this:
{
    // If no task is already running then start one.
    if (_runningLoadTask == null)
    {
        _runningLoadTask = LongRunningQuery();
        await _runningLoadTask; 
    }
    // If a task ist already running then no db call should happen.
    // Instead the currently running task should be awaited.
    else
    {
        await _runningLoadTask; 
        // Alternative way:
        // await Task.WhenAll(_runningLoadTask); 
    }
}
Alois
  • 361
  • 2
  • 18