search

dotnet / interactive

.NET Interactive combines the power of .NET with many other languages to create notebooks, REPLs, and embedded coding experiences. Share code, explore data, write, and learn across your apps in ways you couldn't before.
MIT License
2.8k stars 374 forks source link

ScheduledOperationRunLoop blocks on thread pool tread #3503

Closed bcleech closed 2 months ago

bcleech commented 3 months ago

Describe the bug

While debugging a thread pool starvation issue, I noticed that KernelScheduler.ScheduledOperationRunLoop is executing on a thread pool thread, but spends most of its time blocked (ultimately in Monitor.Wait).

Stack trace when idle:

System.Private.CoreLib.dll!System.Threading.Monitor.Wait(object obj, int millisecondsTimeout) Line 159
System.Private.CoreLib.dll!System.Threading.SemaphoreSlim.WaitUntilCountOrTimeout(int millisecondsTimeout, uint startTime, System.Threading.CancellationToken cancellationToken) Line 444
System.Private.CoreLib.dll!System.Threading.SemaphoreSlim.Wait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Line 365
System.Collections.Concurrent.dll!System.Collections.Concurrent.BlockingCollection<Microsoft.DotNet.Interactive.KernelScheduler<Microsoft.DotNet.Interactive.Commands.KernelCommand, Microsoft.DotNet.Interactive.KernelCommandResult>.ScheduledOperation>.TryTakeWithNoTimeValidation(out Microsoft.DotNet.Interactive.KernelScheduler<Microsoft.DotNet.Interactive.Commands.KernelCommand, Microsoft.DotNet.Interactive.KernelCommandResult>.ScheduledOperation item, int millisecondsTimeout, System.Threading.CancellationToken cancellationToken, System.Threading.CancellationTokenSource combinedTokenSource) Line 700
System.Collections.Concurrent.dll!System.Collections.Concurrent.BlockingCollection<Microsoft.DotNet.Interactive.KernelScheduler<Microsoft.DotNet.Interactive.Commands.KernelCommand, Microsoft.DotNet.Interactive.KernelCommandResult>.ScheduledOperation>.GetConsumingEnumerable(System.Threading.CancellationToken cancellationToken) Line 1659
Microsoft.DotNet.Interactive.dll!Microsoft.DotNet.Interactive.KernelScheduler<Microsoft.DotNet.Interactive.Commands.KernelCommand, Microsoft.DotNet.Interactive.KernelCommandResult>.ScheduledOperationRunLoop(object _) Line 97
System.Private.CoreLib.dll!System.Threading.Tasks.Task.InnerInvoke() Line 2397
System.Private.CoreLib.dll!System.Threading.Tasks.Task..cctor.AnonymousMethod__272_0(object obj) Line 2376
System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(System.Threading.Thread threadPoolThread, System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Line 268
System.Private.CoreLib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot, System.Threading.Thread threadPoolThread) Line 2337
System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch() Line 790
System.Private.CoreLib.dll!System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart() Line 107
System.Private.CoreLib.dll!System.Threading.Thread.StartCallback() Line 106

It looks like it was intended to be started with TaskCreationOptions.LongRunning, but the call to Task.Factory.StartNew in the KernelScheduler constructor is incorrect:

 _runLoopTask = Task.Factory.StartNew(
    ScheduledOperationRunLoop,
    TaskCreationOptions.LongRunning,
    _schedulerDisposalSource.Token);

This actually calls this overload of Task.Factory.StartNew:

public Task StartNew(Action<object> action, object state, CancellationToken cancellationToken)

Which means TaskCreationOptions.LongRunning is passed as the state parameter and is passed as an argument to ScheduledOperationRunLoop (where it is ignored). It should be passed as the creationOptions parameter in one of the other overloads.

Please complete the following:

Which version of .NET Interactive are you using? (In a notebook, run the #!about magic command. ):

1.0.0-beta.22552.6. Not the latest, but issue is still present in latest source code.

Screenshots

If applicable, add screenshots to help explain your problem.

bcleech commented 2 months ago

@jonsequitur , thank you for fixing this. I noticed you no longer pass the CancellationToken to Task.Factory.StartNew. Is this intentional? Would this be better as:

 _runLoopTask = Task.Factory.StartNew(
    ScheduledOperationRunLoop,
    _schedulerDisposalSource.Token,
    TaskCreationOptions.LongRunning,
    TaskScheduler.Current);