I found this method:
public override void OnActionExecuting(HttpActionContext actionContext)
{
var body = Task.Run(async()
=> await actionContext.Request.Content.ReadAsStringAsync()
.ConfigureAwait(false)).Result;
//rest of code omitted for brevity.
}
I'm trying to work out two things:
1.Will this code cause a deadlock?
2.Since the method cannot be marked async Task
, should it be written like this instead?
var body = actionContext.Request.Content.ReadAsStringAsync().ConfigureAwait(false).GetAwaiter().GetResult();
ConfigureAwait(false)
is used to skip any synchronization context and go directly to the thread pool for the continuation work. So I am thinking you are on to something here for the cases where the synchronization context is a problem (single-threaded contexts), such as Windows Forms.The problem, in essence, is that if you lock the synchronization context's only thread with a
.Result
or.Wait()
, then there is no thread for any continuation work to be completed, effectively freezing your application. By ensuring a thread pool thread will complete the job usingConfigureAwait(false)
, then, at least in my head, all should be good as you are suspecting. I think this is a very nice find from you.This however, as pointed out already in the given comments, is not your case for the particular code that you show. For starters, you are explicitly starting a new thread. That alone takes you away from the case you are fearing.
CLARIFICATION: A Task Is Not a Thread
As stated in the comment, I'll clarify. I simplified by saying "starting a new thread" in the last paragraph. I'll elaborate for the sake of the more enlightened.
Whenever you use
Task.Run()
, the newly created task will go sit in a thread pool task queue. There is the global queue, and there is one queue per thread pool thread. This is the fun part:Task.Run()
statement is a thread pool thread, the new task will be placed last in that thread's task queue.Task.Run()
statement is not a thread pool thread, the new task will be placed last in the global task queue.At this point, since we are assuming we are not awaiting and instead we are putting the executing thread to sleep until the task is done, either using
.Result
or.Wait()
, we enter the following situations:If the hill-climbing algoritm is currently creating new threads, or the minimum number of threads has not yet been reached, new thread pool threads will be created. Each of the new threads will dequeue from the global task queue. If no tasks are there, they will proceed to steal work from individual threads' task queues.
This means that, by
.Result
'ing or.Wait()
'ing from a thread has put us in the delicate position of having to wait for another thread pool thread to save us. This is ... brittle? What if the global queue is receiving tasks at a pace faster than the rate new threads are added to the thread pool? Then we have a frozen thread as if it had deadlocked.This is why using
.Result
or.Wait()
is generally discouraged.Anyway, I simplified the way I did because in either case, you have to wait for a different thread to come to your rescue. By writing code like that you have guaranteed that the current thread will never be able to save itself. Hence, "new thread".