Best-way for endless-task

Question

I have a task outputTask. The task reads the output of a StreamReader (with StreamReader.ReadLineAsync()),

It only works in this way:

var outputTask = plinkInstance.StandardOutput.ReadLineAsync().ContinueWith(hasOutputReaded);

Action<Task<string>> hasOutputReaded = null;
hasOutputReaded = (previousTask) =>
{
    resetTimer.Invoke();
    _outputLines.Add(previousTask.Result);
    previousTask = plinkInstance.StandardOutput.ReadLineAsync();
    previousTask.ContinueWith(hasOutputReaded);
};

It works yes, but is there a other way to make this better?

Edit: Now it looks this way

// ...
Task.Factory.StartNew(() => readUntilEnd(plinkInstance.StandardOutput, timer, cancel.Token), cancel.Token);
// ...

And the method

private static void readUntilEnd(StreamReader stream, System.Timers.Timer timer, CancellationToken canelToken)
{
    char[] buffer = new char[1];
    stream.ReadAsync(buffer, 0, 1);
    int cooldown = 0;

    while (!canelToken.IsCancellationRequested)
    {
        if (buffer[0] != '\0')
        {
            readAChar(buffer[0]);
            buffer[0] = '\0';
            stream.ReadAsync(buffer, 0, 1);

            timer.Stop();
            timer.Start();
            cooldown = 0;
         }
         else
         {
             if (cooldown < 100)
                 cooldown++;
         }

          if (cooldown > 0)
              Thread.Sleep(cooldown);
    }
}

Why I need the CancellationToken? I can also pass an bool-object, or not?

decPL · Accepted Answer · 2017-08-31T09:00:07.343

3

Since you're running your continuous task asynchronously, why not just make it an endless loop?

Task.Run(() =>
         {
            while (true)
            {
               var output = await READ_LINE_ASYNCHRONOUSLY();
               _outputLines.Add(output);
            }
         });

And if you need some way to break out of your loop (I'd assume you might), use CancellationToken, as described e.g. here:

How to cancel a Task in await?

EDIT: Here's the full code doing what you probably want to do:

Task.Run(() => ReadUntilEnd(streamReader, cancellationTokenSource.Token),
         cancellationTokenSource.Token);

//...

private static async Task ReadUntilEnd(StreamReader streamReader, 
                                       CancellationToken cancellationToken)
{
    char[] buffer = new char[1];
    while (!cancellationToken.IsCancellationRequested)
    {
        await streamReader.ReadAsync(buffer, 0, 1);
        readAChar(buffer[0]);
    }
}

edited Aug 31 '17 at 09:00

answered Aug 30 '17 at 11:44

decPL

5,384
1
26
36

The task is (at my first attempt) a local variable. So the GarbageCollector stops the loop (I guess). But here I have to write an additional cancellation. – akop Aug 30 '17 at 11:59
2

No, GC won't stop your loop, as your task keeps referring itself, so it won't be viable for garbage collection. It will run for as long as your app (or thread, if you're doing something more complicated) is running. I've also simplified my example - if for some reason you want to have a variable instead of providing the Task.Run() argument inline, you can do it. But it will still do nothing with respect to how and when the task is terminated. – decPL Aug 30 '17 at 12:06
Is the new way better? (Look at first post) – akop Aug 30 '17 at 14:41
@user6537157 Yes in terms that it's a specific implementation of the generic one I've written above. No, because you're firing asynchronous methods (`ReadAsync`) without waiting for a result, but you're trying to use the result still (by accessing the `buffer` variable). Bedtime reading: https://msdn.microsoft.com/library/hh191443(vs.110).aspx , but the short version is: 1. make your `readUntilEnd` method `async Task` (instead of `void`) 2. add the `await` keyword before each call to `ReadAsync` – decPL Aug 31 '17 at 07:45
@user6537157 A significant problem with your code is that it will not work in a deterministic way - it might even work correctly some time, but at times it will miss some lines and read the same line twice, because of how your code progresses independently of the stream reading part. – decPL Aug 31 '17 at 07:48
Why it will miss lines or read the same twice? All tests are fine. Thanks for the hint with `await`-statement. – akop Aug 31 '17 at 07:59
Sorry, not line, but character (if you won't use the async-await pattern, or some other way to synchronize your code). Imagine the following scenario - you're reading through a line, say you've read the 3rd character. Now you're running ReadAsync to read the 4th character; say the synchronizer decides to run this on a separate thread, but your thread takes priority and you loop back to the line reading from your `buffer` variable. Which still holds the 3rd character, so you happily read that. Also - if you use async-await, use `await Task.Delay` instead of `Thread.Sleep` (cont.) – decPL Aug 31 '17 at 08:21
(cont.) or better yet - scrap the whole sleeping bit altogether - that's what await will do for you implicitly when there's nothing new in the buffer. – decPL Aug 31 '17 at 08:22
Ok, I rewrote the method. Not any of `ReadAsync()` overloads take a `CancellationToken`, so I made it this way. What do you think? (Look at second answer) – akop Aug 31 '17 at 08:37
Updated my answer. – decPL Aug 31 '17 at 09:00

score 0 · Answer 2 · answered Aug 31 '17 at 08:35

private static async Task readUntilEnd(StreamReader stream, System.Timers.Timer timer, CancellationToken canelToken)
{
    char[] buffer = new char[1];
    Task readTask = stream.ReadAsync(buffer, 0, 1);
    int cooldown = 5;

    while (!canelToken.IsCancellationRequested)
    {
        if (readTask.IsCompleted)
        {
             readAChar(buffer[0]);

             buffer[0] = '\0';
             readTask = stream.ReadAsync(buffer, 0, 1);

             timer.Stop();
             timer.Start();

             cooldown = 0;
        }
        else
        {
            if (cooldown < 100)
                cooldown++;
        }

        if (cooldown > 0)
            await Task.Delay(cooldown, canelToken);
    }

}

Best-way for endless-task

2 Answers2