0

I have a particular problem where my Starting of the Task (Line 13) never returns. But i dont want to await this. (and for propper cleanup waiting i want the refrence)

My Code is here: https://pastebin.com/4Zb1bzJX

Some Say this call should return, others say use "ConfiguraAwait"(no option since i will not be able to get the instance)

I am unsure how to do it the "right way". Just get it working would be easy with a _bufferWorker = Task.Factory.StartNew(()=> BufferWorker()); but this just feels hacky and not right, since just creating a delegate to invoke something that is already a Task seems wrong)

How should i go about this problem there? (Maybe Tasks arent even the way i want to go here, and just use old Threads?)

Full included code to make it more readable:

    private async Task BufferWorker()
    {
        while (_working)
        {
            if(!_queue.TryDequeue(out var data)) continue;
            await Task.Delay(_provider.BufferedDuration);
            _provider.AddSamples(data, 0, data.Length);
        }
    }

    public void Open()
    {
        _working = true;
        _bufferWorker = BufferWorker();//this line never returns
    }
    public void Close()
    {
        _working = false;
        _bufferWorker.Wait();
        _bufferWorker = null;
    }

EDIT: i added a comment on the line that never returns

IMPORTANT INFORMATION: i also added a line that i stripped=> if TryQequeue fails i continue the loop. This is important, since the Task will return if i hit the first await. With this i can solve(by just waiting 1 ms on top), but this would be also a hack.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Ranndom
  • 118
  • 12
  • 2
    Please edit your question to include the code directly - it makes it significantly easier for people to help you, and for others who arrive at the question later to understand the question without having to go to another site. – Jon Skeet Jul 04 '21 at 18:23
  • @GSerg not really, since its exactly the way i dont want to do (wrap the task method in a delegate.) your solution is just more safe against wrong uses. So i learned something from it, but i dont think this is what i am looking for. (but i am unsure if what i want to do is even propperly doable, thats why i "self suggested" if threads maybe better for this) – Ranndom Jul 04 '21 at 18:39
  • Can you point the line of code that never returns? Maybe add a code-comment in that line, like: `// this line never returns` – Theodor Zoulias Jul 04 '21 at 18:50
  • @TheodorZoulias i added this comment. (and an information for this too). just to make sure that not this will be the next thing suggested. BUT basicly my question was not about the line by line code, more about the correct approach. but as it seems this got lost due to me not being a native english speaker. – Ranndom Jul 04 '21 at 18:55
  • When you say "get the instance" do you mean "get the result"? – Caius Jard Jul 04 '21 at 18:59
  • @CaiusJard there is no result. its a running "worker". by instance i mean the task instance (as seen in the "Close" method i exit the loop with "_working = false" and wait for the task to be finished directly in the line after) – Ranndom Jul 04 '21 at 19:03
  • Can you also add the definition of the `_queue` variable? Is it a `Queue`? Also what is the value of the `_provider.BufferedDuration`? Is it `0` by any chance? – Theodor Zoulias Jul 04 '21 at 19:14
  • @TheodorZoulias the type of this queue is completly irrelevant. Thanks for your try. the answer accepted below satisfies my question. – Ranndom Jul 04 '21 at 19:43
  • 2
    You were accepted an answer that probably a bad-practice container. I don't suggest to use that code. – aepot Jul 04 '21 at 21:39

2 Answers2

-1

TryDequeue that immediately returns null when no items in queue + Thread.Sleep(FixedTime) will make BufferWorker work with some delays. It is better to use Windows/.Net built-in synchronization primitives (Monitor/Event) or data structures to make BufferWorker work instantly:

BlockingCollection<TData> _queue = new BlockingCollection<TData>(10);
private void BufferWorker(CancellationToken ct)
{
    try {
        foreach (var data in _queue.GetConsumingEnumerable(ct))
            _provider.AddSamples(data, 0, data.Length);
    }
    catch (OperationCanceledException) { } // exit requested
}

CancellationTokenSource _cts;
public void Open()
{
    _cts = new CancellationTokenSource();
    new Thread(x => BufferWorker(_cts.Token)).Start();
}
public void Close()
{
    _cts.Cancel();
}
GraphWalk
  • 319
  • 3
  • 9
  • Why do you avoiding both asynchronous programming and Thread Pool? Creating new foreground Thread for each call may cause either nover-closing app, or performance issues. Try learn something at least about `Task.Run`, it does almost the same but sligtly better than shown in the answer. The good point here is `CancellationToken`, I like it. – aepot Jul 04 '21 at 21:45
-3

If you call an async method without await, it will implicitly wait for the result. Since the task never ends, it will wait forever. You can either use the Factory.StartNew-approach to work around this or use a Thread instead. For jobs that run for extended times, particularly ones that have their own while(_working) loop, a Thread is usually the better choice than a task.

private void BufferWorker()
    {
        while (_working)
        {
            if(!_queue.TryDequeue(out var data)) continue;
            Thread.Sleep(_provider.BufferedDuration);
            _provider.AddSamples(data, 0, data.Length);
        }
    }

    public void Open()
    {
        _working = true;
        _bufferWorker = new Thread(BufferWorker);
        _bufferWorker.Start();

    }
    public void Close()
    {
        _working = false;
        _bufferWorker.Join();
        _bufferWorker = null;
    }

Note that you still need to insert some sleep before the continue, or this will do busy waiting.

PMF
  • 14,535
  • 3
  • 23
  • 49
  • That was what is suspected. Thank you for this confirmation. (PS: the sleep before continue is true, but my code was stripped down to not bloat all this here.) – Ranndom Jul 04 '21 at 19:06
  • 3
    You are very wrong in the claim "If you call an `async` method without `await`, it will implicitly wait for the result." Example proving you wrong: https://dotnetfiddle.net/tNK2fs. There is of course a lot going on, but basic rule of thumb would be that as soon as the first `await` is encountered within that `async` method it returns and resumes where you called the "`async` method without `await`". –  Jul 04 '21 at 19:54
  • @Knoop Good catch, that confuses me a bit now. No really useful situation, though. – PMF Jul 04 '21 at 20:15
  • Well it's important to know what happens. When you call an `async` method it stays on the same thread until an `await` is encountered. Knowing that, my assumption would be that OP's problem is that the calling thread is also the thread that should do the enqueue's but it can never do it because it's stuck on an infinite loop trying to dequeue an empty queue (it never hits the `await` so stays on the calling thread). –  Jul 04 '21 at 20:25
  • One last note, for these scenario's there are overloads for `TaskFactory.StartNew` that lets you pass `TaskCreationOptions.LongRunning`, hinting the scheduler that it would be good to start a new thread for this task. (Prevents doing the thread management yourself, without blocking the task thread pool) –  Jul 04 '21 at 20:27
  • 2
    This is a combination of [wrong information](https://stackoverflow.com/q/46053175/11683), [bad advice](https://blog.stephencleary.com/2013/08/startnew-is-dangerous.html) and bad code. – GSerg Jul 04 '21 at 21:14
  • @PMF please read [Asynchronous programming](https://learn.microsoft.com/en-us/dotnet/csharp/async). – aepot Jul 04 '21 at 21:37