Starting async method as Thread or as Task
Asked Answered
F

5

21

I'm new to C#s await/async and currently playing around a bit.

In my scenario I have a simple client-object which has a WebRequest property. The client should send periodically alive-messages over the WebRequests RequestStream. This is the constructor of the client-object:

public Client()
{
    _webRequest = WebRequest.Create("some url");
    _webRequest.Method = "POST";

    IsRunning = true;

    // --> how to start the 'async' method (see below)
}

and the async alive-sender method

private async void SendAliveMessageAsync()
{
    const string keepAliveMessage = "{\"message\": {\"type\": \"keepalive\"}}";
    var seconds = 0;
    while (IsRunning)
    {
        if (seconds % 10 == 0)
        {
            await new StreamWriter(_webRequest.GetRequestStream()).WriteLineAsync(keepAliveMessage);
        }

        await Task.Delay(1000);
        seconds++;
    }
}

How should the method be started?

new Thread(SendAliveMessageAsync).Start();

or

Task.Run(SendAliveMessageAsync); // changing the returning type to Task

or

await SendAliveMessageAsync(); // fails as of the constructor is not async

My question is more about my personal understanding of await/async which I guess may be wrong in some points.

The third option is throwing

The 'await' operator can only be used in a method or lambda marked with the 'async' modifier
Frijol answered 15/1, 2016 at 9:31 Comment(5)
First you have to decide whether this is a fire-and-forget method or something you want to wait for.Bluetongue
It is indeed a fire-and-forget as it simply should start a Thread that send alive messages.Frijol
Then await is pointless as you're not really interested in waiting for it to complete. I would simply use the thread approach then. Personally I consider tasks as smaller contained units of work that needs to be run asynchronously.Bluetongue
You should never call new StreamWriter(_webRequest.GetRequestStream()).WriteLineAsync(keepAliveMessage) - await or otherwise - as StreamWriter is IDisposable and must be properly disposed of once used. This syntax doesn't allow you to call .Dipose().Steradian
@Steradian Good and valid point. But this is not the working code. The StreamWriter is also initialized in the constructor but for simplicity reasons not shown here.Frijol
N
13

How should the method be started?

I vote for "none of the above". :)

"Fire and forget" is a difficult scenario to handle correctly. In particular, error handling is always problematic. In this case, async void may surprise you.

I prefer to explicitly save the tasks if I'm not awaiting them immediately:

private async Task SendAliveMessageAsync();

public Task KeepaliveTask { get; private set; }

public Client()
{
  ...
  KeepaliveTask = SendAliveMessageAsync();
}

This at least allows the consumers of Client to detect and recover from exceptions thrown by the SendAliveMessageAsync method.

On a side note, this pattern is almost equivalent to my "asynchronous initialization" pattern.

Nickynico answered 23/1, 2016 at 4:44 Comment(0)
U
9

Edited as previous answer was wrong:

As it's in the constructor, I think you would have to spin up a new thread for it. I would personally do that using

Task.Factory.StartNew(() => SendAliveMessageAsync());

Unhopedfor answered 15/1, 2016 at 9:34 Comment(3)
The code is being run in the constructor. Calling await doesn't work in constructors.Steradian
@JohnClifford Is there any difference between Task.Run and Task.Factory.StartNew? Or is this just your personal opinion?Frijol
To be honest I use Task.Factory.StartNew out of force of habit; Task.Run is pretty much a shortcut for it, so if you'd rather use that go right ahead.Unhopedfor
S
1

Here's an option since you can't call await from inside a constructor.

I would suggest using Microsoft's Reactive Framework (NuGet "Rx-Main").

The code would look like this:

public class Client
{
    System.Net.WebRequest _webRequest = null;
    IDisposable _subscription = null;

    public Client()
    {
        _webRequest = System.Net.WebRequest.Create("some url");
        _webRequest.Method = "POST";

        const string keepAliveMessage = "{\"message\": {\"type\": \"keepalive\"}}";

        var keepAlives =
            from n in Observable.Interval(TimeSpan.FromSeconds(10.0))
            from u in Observable.Using(
                () => new StreamWriter(_webRequest.GetRequestStream()),
                sw => Observable.FromAsync(() => sw.WriteLineAsync(keepAliveMessage)))
            select u;

        _subscription = keepAlives.Subscribe();
    }
}

This code handles all of the threading required and properly disposes of the StreamWriter as it goes.

Whenever you want to stop the keep alives just call _subscription.Dispose().

Steradian answered 15/1, 2016 at 9:52 Comment(0)
M
1

In your code there is no need to using async/await, just set up a new thread to perform long operation.

private void SendAliveMessage()
{
    const string keepAliveMessage = "{\"message\": {\"type\": \"keepalive\"}}";
    var sreamWriter = new StreamWriter(_webRequest.GetRequestStream());
    while (IsRunning)
    {
        sreamWriter.WriteLine(keepAliveMessage);
        Thread.Sleep(10 * 1000);
    }    
}

Using Task.Factory.StartNew(SendAliveMessage, TaskCreationOptions.LongRunning) to perform the operation.

If you really want to using async/await pattern, just call it in constructor without await modifier and the forget it.

public Client()
{
    _webRequest = WebRequest.Create("some url");
    _webRequest.Method = "POST";

    IsRunning = true;

    SendAliveMessageAsync();    //just call it and forget it.
}

I think it's not good idea to set up a long running thread or using async/await pattern. Timers maybe more suitable in this situation.

Marconigraph answered 15/1, 2016 at 9:58 Comment(1)
Don't ever call logic in a constructor. Especially asynchronous logic. Constructors are supposed to be short lived and cannot be async.Trelu
S
0

Since it is a fire and forget operation, you should start it using

SendAliveMessageAsync();

Note that await does not start a new Task. It is just syntactical sugar to wait for a Task to complete.
A new thread is started using Task.Run.

So inside SendAliveMessageAsync you should start a new Task:

private async Task SendAliveMessageAsync()
{
    const string keepAliveMessage = "{\"message\": {\"type\": \"keepalive\"}}";
    await Task.Run( () => {
        var seconds = 0;
        while (IsRunning)
        {
            if (seconds % 10 == 0)
            {
                await new StreamWriter(_webRequest.GetRequestStream()).WriteLineAsync(keepAliveMessage);
            }

            await Task.Delay(1000);
            seconds++;
        }
    });
}
Sadism answered 15/1, 2016 at 9:34 Comment(4)
The code is being run in the constructor. Calling await doesn't work in constructors.Steradian
But is calling it with SendAliveMessageAsync(); not synchronously and therefore the Constructor will never return (because of the methods while)?Frijol
@Steradian oh wow, how could I miss that. Thanks for the reminderSadism
@Frijol - Yes, that loop in a constructor is bad. Constructors should complete asap and should never risk a exception being thrown.Steradian

© 2022 - 2024 — McMap. All rights reserved.