3

I have the following code:

myObject object1 = null;
Thread obj1Thread = new Thread(() => { object1 = _myService.GetMethod(variable1, variable2); });
obj1Thread.Start();
obj1Thread.Join();


myObject object2 = null;
Thread obj2Thread = new Thread(() => { object2 = _myService.GetMethod2(variable3, variable4); });
obj2Thread.Start();
obj2Thread.Join();

As far as I understand, this code will create 2 new threads, run the specified methods, pause the main thread until both these threads complete, and then continue execution.

Assuming what I say is correct, all fine so far.

Next I want to try this:

myObject object1 = null;
Thread obj1Thread = new Thread(async () => { object1 = await _myService.GetMethod(variable1, variable2); });
obj1Thread.Start();
obj1Thread.Join();


myObject object2 = null;
Thread obj2Thread = new Thread(async () => { object2 = await _myService.GetMethod2(variable3, variable4); });
obj2Thread.Start();
obj2Thread.Join();

Basically adding async and await to each thread.

The compiler accepts this change and it seems to run locally, but is this code ok, and is it likely to cause me any problems further down the line, for example will the threads get confused, fail to wait, mix up results etc.

I have a reasonably good understanding of async and a basic understanding of multi threading, and I cannot think of any reason why this would not work.

The code runs locally, but my worry is that under heavy load on the server issues may appear that were not present in a local version....

Improve this question
10
  • 1
    Why do you need the threads? Isn't async enough? Commented Jan 14, 2019 at 11:33
  • @tkausl this is a page load API and there are many actions going on at once. If I queue them they will take 3+ seconds. If I run them concurrently they will take less than half a second. I am trying to make my server more efficient by using async. Commented Jan 14, 2019 at 11:35
  • 2
    Your starting example just feels wrong - start a new thread to run some code then immediately pause the original thread until the new thread exits. Then rinse and repeat with the second thread. This has exactly the same behaviour as if you've just written the two GetMethod calls directly and never touched threads. Commented Jan 14, 2019 at 11:40
  • @Damien_The_Unbeliever can you explain why? As far as I understand it, the new threads will execute their code until they reach they I/O, at which point they will release their thread until the I/O has finished and then complete their task. The scenario you describe sound very different to that.... Commented Jan 14, 2019 at 11:42
  • 1
    Threads make sense for CPU bound activity, especially when you're trying to provide some degree of "fairness". If your workload is I/O bound (network, disk, database) then threading may not help and could make things worse. Those kind of workloads benefit from async structuring. Commented Jan 14, 2019 at 11:44

3 Answers 3

Reset to default
3

The compiler accepts this change and it seems to run locally, but is this code ok, and is it likely to cause me any problems further down the line, for example will the threads get confused, fail to wait, mix up results etc.

I have a reasonably good understanding of async and a basic understanding of multi threading, and I cannot think of any reason why this would not work.

Yes, this code will cause you problems. The threads are not waiting as you expect. You're passing an async void lambda to the Thread constructor, and that thread will exit as soon as it hits the await in that lambda, before it sets the object1/object2 variable. So it's entirely possible those variables remain null after the Join.

The proper solution, as FCin posted, is to use asynchronous concurrency. (I avoid the term "parallel" here to reduce confusion with the Parallel type and the Task Parallel Library). Asynchronous concurrency uses Task.WhenAll:

// Start both methods concurrently
var task1 = _myService.GetMethod(variable1, variable2);
var task2 = _myService.GetMethod2(variable3, variable4);

// (Asynchronously) wait for them both to complete.
await Task.WhenAll(task1, task2);

// Retrieve results.
myObject object1 = await task1;
myObject object2 = await task2;
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

Your code can be parallel and asynchronously awaited with a single line: 

await Task.WhenAll(_myService.GetMethod(variable1, variable2), _myService.GetMethod2(variable3, variable4)).

That's all you need. No threads. No joining. If these methods are truely i/o, there will be no thread.

As always, must read: https://blog.stephencleary.com/2013/11/there-is-no-thread.html

If your methods return different results and need to be assigned to variables, then you can do this:

Task<int> task1 = _myService.GetMethod(variable1, variable2);
Task<string> task2 = _myService.GetMethod2(variable3, variable4);
// Both tasks started.

int a = await task1; // Wait for first task
string b = await task2; // If this already finished, it will just return the result
Improve this answer

2 Comments

how about when my code assigns a value to a variable within the lambda function?
@Alex Then you can start both methods, so they execute in parallel and then wait for each of them and assign to appropriate variables.
0

Your above code doesn't take advantage of threading, it blocks the first thread until it finishes then starts the second

obj1Thread.Join(); instructs your main thread to wait until obj1Thread exits before continuing. This means it will spin up obj1Thread then wait for it to complete, meaning you will:

  1. create thread 1
  2. Run thread 1
  3. Exit thread 1
  4. create thread 2
  5. Run thread 2
  6. Exit thread 2

You want to do:

myObject object1 = null;
Thread obj1Thread = new Thread(async () => { object1 = await _myService.GetMethod(variable1, variable2); });
obj1Thread.Start();
myObject object2 = null;
Thread obj2Thread = new Thread(async () => { object2 = await _myService.GetMethod2(variable3, variable4); });
obj2Thread.Start();
obj1Thread.Join();
obj2Thread.Join();
Improve this answer

3 Comments

You're correct but speaking generically it's best to sub-thread them as there's almost always some amount of processing you can be doing with the main thread. It just isn't shown in this particular case.
You are basically creating 2 new threads to suspend them almost immediately.
Depends on how long each service takes to complete. In the case of a single IO read it's a bit much but if there's processing involved after the async read it's well worth it. Again this is talking from a general perspective regarding OP's question.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.