I know that Task.Run runs on a threadpool thread and threads can have re-entrancy. But I never knew that 2 tasks can run on the same thread when they are both alive!
The phenomenon is: on the same thread, execute code in Task 1 -> Execute code in Task 2 -> Again execute code on Task 1.
My Question is: is that reasonable by design? Does that mean lock inside an async method is meaningless?
IMO, at least for a piece of sync code, it should be executed all or none on one thread, you cannot cut in the middle and jump to another task.
Suppose I have a function which uses lock to avoid multi-thread access:
void SyncMethod() {
lock(myLock) {
// Do some sync work
}
}
and 2 tasks:
Task.Run(async() => {
// Do some async work...
SyncMethod();
});
Task.Run(async() => {
// Do some async work...
SyncMethod();
})
Because these 2 tasks could run on the same thread, myLock doesn't work at all (because lock allows re-entrance on the same thread)!
My worry is NOT ill-founded, because it is exactly what I met in my code!
For demonstration, I simplify the situation: I create 2 tasks, one is a monitor task and the other is a work task.
I have 3 buttons, to start, pause and terminated the work task respectively.
First I click start button, and then click the pause button, after several seconds, I click the terminate button. The output is :
Work Start on thread: 4
Before set machine state to pause by monitor on thread: 3
After set machine state to pause by monitor on thread: 3
Before set machine state to terminate by monitor on thread: 3
Work exit on thread 3
After set machine state to terminate by monitor on thread: 3
The corresponding code of the last 3 outputs is:
1. Monitor task:
Console.WriteLine($"Before set machine state to terminate by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
2. Jumps to Work task:
Console.WriteLine($"Work exit on thread {Thread.CurrentThread.ManagedThreadId}");
3. Bask to Monitor Task:
machine.TrySetState(MachineState.Terminate);
Console.WriteLine($"After set machine state to terminate by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
Note that there're no async lines above!
Part of my code is pasted below, and complete code can be found at https://gist.github.com/dongmingsun/90bb9131f99e925d27a0d04665ca814a
public partial class Form1 : Form
{
private Machine machine = new Machine();
private bool minorErrorOccurs = false;
private bool majorErrorOccurs = false;
public Form1()
{
InitializeComponent();
StartMonitor();
}
private void btnStart_Click(object sender, EventArgs e)
{
machine.TrySetState(MachineState.Running);
Task taskDoWork = Task.Run(async () =>
{
try
{
await machine.DoWork();
}
catch (Exception ex)
{
}
finally
{
Console.WriteLine($"Work exit on thread {Thread.CurrentThread.ManagedThreadId}");
Thread.Sleep(1000); // Imitate some clean-up work
machine.TrySetState(MachineState.Ready);
}
});
}
private void btnMinorError_Click(object sender, EventArgs e)
{
minorErrorOccurs = true;
}
private void btnMajorError_Click(object sender, EventArgs e)
{
majorErrorOccurs = true;
}
/// <summary>
/// Monitor if errors occur in the "machine"
/// </summary>
private void StartMonitor()
{
Task.Run(async () =>
{
while (true)
{
await Task.Delay(1000);
if (minorErrorOccurs)
{
minorErrorOccurs = false;
Console.WriteLine($"Before set machine state to pause by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
machine.TrySetState(MachineState.Pause);
Console.WriteLine($"After set machine state to pause by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
}
if (majorErrorOccurs)
{
majorErrorOccurs = false;
Console.WriteLine($"Before set machine state to terminate by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
machine.TrySetState(MachineState.Terminate);
Console.WriteLine($"After set machine state to terminate by monitor on thread: {Thread.CurrentThread.ManagedThreadId}");
}
}
});
}
}
CodePudding user response：
Use SemaphoreSlim instead of lock, since, as the documentation says:
The SemaphoreSlim class doesn't enforce thread or task identity
In your case, it would look something like this:
// Semaphore only allows one request to enter at a time
private static readonly SemaphoreSlim _semaphoreSlim = new SemaphoreSlim(1, 1);
void SyncMethod() {
_semaphoreSlim.Wait();
try {
// Do some sync work
} finally {
_semaphoreSlim.Release();
}
}
The try/finally block is optional, but it makes sure that the semaphore is released even if an exception is thrown somewhere in your code.
Note that SemaphoreSlim also has a WaitAsync() method, if you want to wait asynchronously to enter the semaphore.
CodePudding user response：
A task is an abstraction over some amount of work. Usually this means that the work is split into parts, where the execution can be paused and resumed between parts. When resuming it may very well run on another thread. But the pausing/resuming may only be done at the await statements. Notably, while the task is 'paused', for example because it is waiting for IO, it does not consume any thread at all, it will only use a thread while it is actually running.
My Question is: is that reasonable by design? Does that mean lock inside an async method is meaningless?
locks inside a async method is far from meaningless since it allows you to ensure a section of code is only run by one thread at a time.
In your first example there can be only one thread that has the lock at a time. While the lock is held that task cannot be paused/resumed since await is not legal while in a lock body. So a single thread will execute the entire lock body, and that thread cannot do anything else until it completes the lock body. So there is no risk of re-entrancy.
I'm not sure what your second example is trying to prove, but I cannot see any locks, so I'm not sure it is relevant.