Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Can I use task.Wait(); like that? Note that when I call task.Wait the task is probably already finished.

And probably you can suggest better pattern.

class A {
    private Task task1;
    private Task task2;

    ...
    public void Connect() {
        stayConnected = true;
        task1 = Task.Factory.StartNew(....,
            while (stayConnected) {
                ....
            }
            LongRunning);
        task2 = Task.Factory.StartNew(....,
            while (stayConnected) {
                ....
            }
            LongRunning);
    }

    private volatile bool stayConnected;

    // should be synchronous. when return everything should be disconected
    public void Disconnect() {
        stayConnected = false;
        task1.Wait();
        task1 = null;
        task2.Wait();
        task2 = null;
    }

}
share|improve this question
add comment

3 Answers

You can use a CancellationToken and Task.WaitAll(...) to do this...

public class A
{
    private readonly CancellationTokenSource tokenSource = new CancellationTokenSource();
    private Task task1;
    private Task task2;

    public void Connect()
    {
        task1 = Task.Factory.StartNew(SomeWork, tokenSource.Token);
        task2 = Task.Factory.StartNew(SomeWork, tokenSource.Token);
    }

    public void Disconnect()
    {
        tokenSource.Cancel();
        Task.WaitAll(task1,task2);
    }

    public void SomeWork(object o)
    {
        CancellationToken token = (CancellationToken)o;
        while (!token.IsCancellationRequested)
        {
            Console.WriteLine("working...");
        }
    }
}

For more information on managed threads check this and this article on MSDN. Enjoy :D

share|improve this answer
    
why cancellationtoken is better than just "volatile boolean"? in general i prefer "easy to port to c++" code. –  javapowered Nov 29 '12 at 15:54
    
I think CancellationToken should be used when you want to cancel a Task prematurely. I think that's not the case here. Also, canceling the Task properly (which you didn't do) would mean WaitAll() would throw an exception. –  svick Nov 29 '12 at 16:39
    
You can certainly use volatile if you want to keep it simple. –  Alechandro Nov 29 '12 at 16:48
    
@svick why does it throw an exception? –  Alechandro Nov 29 '12 at 17:01
    
@Alechandro Because the Tasks would be canceled. Wait() and WaitAll() throw an exception when you wait on a canceled Task. –  svick Nov 29 '12 at 17:51
add comment

Task.Wait() should just return true if the task is completed, so sure you can. However, you should better use waiting with timeout or TimeSpan parameter if you have actions inside of while { } loop that can possibly cause a freeze.

share|improve this answer
    
Task.Wait() without a timeout doesn't return anything. –  svick Nov 29 '12 at 18:41
add comment

This seems like a reasonable approach to me. You could improve it a bit by using Task.WaitAll() but that won't change much.

Also, I wouldn't set the fields to null unless I had good reason to do it. Yeah, those Tasks are not useful for anything anymore, but they are also almost free.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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