-
-
Notifications
You must be signed in to change notification settings - Fork 357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
StackOverflowException and Thread stealing when combining LockAsync and sync code #36
Comments
Yes, I think your fix is appropriate. I do have to double-check some related semantics and make sure. On a side note, the .NET Core version of AsyncEx primitives do always use asynchronous continuations. |
Hi! Is there an update on this? I am having that issue I believe (and the same StackOverflow with stack trace from windbg). Or perhaps a work-around? Sorry, I couldn't quite figure it out from the linked SO post. |
@fry: The new version of the async coordination primitives is available in a prerelease version. There have been some (minor) API changes, but most upgrades should go quite smoothly. |
I ran into this in my codebase a few times. I haven't switched to the new API yet but will do so in a few weeks when I can have things broken for a few days (testing for this sort of thing is hard as it is tricky to reproduce). In my code I had a whole bunch of repositories wait on a single shared async lock on startup - it was to allow initialisation to happen exactly once (but not too soon). Those repositories could have 100 or so clients waiting to make their first request. Rather than have so many waiters on the one async lock, I added another async lock and a flag to each repository so the queue of waiters was spread amongst more locks. That solved the problem for me. At one stage I experimented with awaiting Task.Yield() too. I'm not entirely sure that helped as I'm not familiar enough with the internals of how that might interact with the call stack and continuations. Note that I'm not really making any use of ConfigureAwait() anywhere. That might have an effect too as far as call stack depth is concerned. |
I am having similar trouble running 3.0.1. I have the following wrapper class around AsyncMonitor public class EasyAsyncMonitor
{
private AsyncMonitor _monitor = new AsyncMonitor();
public Task WaitAsync(CancellationToken token = default(CancellationToken))
{
return EnterMonitorAndDo(() =>
{
return _monitor.WaitAsync();
}, token);
}
public Task Pulse(CancellationToken token = default(CancellationToken))
{
return EnterMonitorAndDo(() =>
{
_monitor.Pulse();
}, token);
}
public Task PulseAll(CancellationToken token = default(CancellationToken))
{
return EnterMonitorAndDo(() =>
{
_monitor.PulseAll();
}, token);
}
private async Task EnterMonitorAndDo(Func<Task> doThis, CancellationToken token)
{
using(await _monitor.EnterAsync(token))
{
await doThis().DetachContext(); //Overflow occurs here
}
}
private async Task EnterMonitorAndDo(Action doThis, CancellationToken token)
{
using(await _monitor.EnterAsync(token))
{
doThis();
}
}
} And DetachContext is defined as public static ConfiguredTaskAwaitable DetachContext(this Task t)
{
return t.ConfigureAwait(false);
} Occasionally I will get a stackoverflow exception thrown from the line indicated above with the comment. The stack trace is unavailable but is visible in Visual studio debugging. It is full of that single line and then [Resuming Async Method] and [External code] entries between them. Could this be related or is it incorrect usage of the Monitor? |
Should be fixed in |
When using AsyncLock.LockAsync() in combination with only synchronous running code a StackOverflowException can happen during dispose/unlock.
The following code snippet runs into the StackOverflow:
Will result in the following stack (taken from windbg - read bottom up):
Shouldn't the call to
TaskCompletionSource<T>.TrySetResult
in theCompleteDisposable
be replaced with either an offload likeTrySetResultWithBackgroundContinuations
despite the performance implications or one of the possibilities from this SO thread (as far as I remember you participated in there)The text was updated successfully, but these errors were encountered: