diff --git a/dotnet/CodeExec.Roslyn/src/GuardWriter.cs b/dotnet/CodeExec.Roslyn/src/GuardWriter.cs index 20f0d63..98d48ba 100644 --- a/dotnet/CodeExec.Roslyn/src/GuardWriter.cs +++ b/dotnet/CodeExec.Roslyn/src/GuardWriter.cs @@ -265,6 +265,9 @@ void Add(string type, string method, string guard, params string[][] overloads) foreach (var pair in new[] { new[] { "SemaphoreSlim", "SemaphoreWait" }, new[] { "ManualResetEventSlim", "EventWait" }, new[] { "CountdownEvent", "CountdownWait" } }) Add("System.Threading." + pair[0], "Wait", pair[1], none, new[] { Ms }, new[] { Span }, new[] { Token }, new[] { Ms, Token }, new[] { Span, Token }); Add("System.Threading.Monitor", "Wait", "MonitorWait", new[] { "object" }, new[] { "object", Ms }, new[] { "object", Span }); + // Not waits, but the sliced Monitor.Wait needs them: a pulse while its waiter is between two slices is not lost. + Add("System.Threading.Monitor", "Pulse", "MonitorPulse", new[] { "object" }); + Add("System.Threading.Monitor", "PulseAll", "MonitorPulseAll", new[] { "object" }); Add("System.Threading.WaitHandle", "WaitOne", "WaitOne", none, new[] { Ms }, new[] { Span }); Add("System.Threading.WaitHandle", "WaitAll", "WaitAll", new[] { HandleArray }, new[] { HandleArray, Ms }, new[] { HandleArray, Span }); Add("System.Threading.WaitHandle", "WaitAny", "WaitAny", new[] { HandleArray }, new[] { HandleArray, Ms }, new[] { HandleArray, Span }); diff --git a/dotnet/CodeExec.Tests/GuardTests.cs b/dotnet/CodeExec.Tests/GuardTests.cs index 6859c7e..7ee87a1 100644 --- a/dotnet/CodeExec.Tests/GuardTests.cs +++ b/dotnet/CodeExec.Tests/GuardTests.cs @@ -573,6 +573,12 @@ public void GuardsCanBeTurnedOff() [InlineData("Task.WaitAll(new[] { Task.Delay(10_000) }, 120)", "False")] [InlineData("System.Threading.WaitHandle.WaitAny(new System.Threading.WaitHandle[] { new System.Threading.ManualResetEvent(false) }, 120)", "258")] [InlineData("var o = new object();\nvar t = new System.Threading.Thread(() => { System.Threading.Thread.Sleep(200); lock (o) System.Threading.Monitor.Pulse(o); });\nbool got;\nlock (o) { t.Start(); got = System.Threading.Monitor.Wait(o); }\nt.Join();\ngot", "True")] + // A Pulse while a sliced waiter is between two slices (the pulsing thread holds the lock longer than a slice) still + // wakes it, and one Pulse wakes one waiter. + [InlineData("var o = new object();\nvar woken = 0;\nvar entered = new System.Threading.CountdownEvent(2);\nvar all = new System.Collections.Generic.List();\nfor (var i = 0; i < 2; i++) { var t = new System.Threading.Thread(() => { lock (o) { entered.Signal(); if (System.Threading.Monitor.Wait(o, 2000)) woken++; } }); t.Start(); all.Add(t); }\nentered.Wait();\nlock (o) { System.Threading.Thread.Sleep(200); System.Threading.Monitor.Pulse(o); }\nforeach (var t in all) t.Join();\nwoken", "1")] + [InlineData("var o = new object();\nvar woken = 0;\nvar entered = new System.Threading.CountdownEvent(2);\nvar all = new System.Collections.Generic.List();\nfor (var i = 0; i < 2; i++) { var t = new System.Threading.Thread(() => { lock (o) { entered.Signal(); if (System.Threading.Monitor.Wait(o, 2000)) woken++; } }); t.Start(); all.Add(t); }\nentered.Wait();\nlock (o) { System.Threading.Thread.Sleep(200); System.Threading.Monitor.PulseAll(o); }\nforeach (var t in all) t.Join();\nwoken", "2")] + [InlineData("var o = new object();\nvar woken = 0;\nvar entered = new System.Threading.CountdownEvent(3);\nvar all = new System.Collections.Generic.List();\nfor (var i = 0; i < 3; i++) { var t = new System.Threading.Thread(() => { lock (o) { entered.Signal(); if (System.Threading.Monitor.Wait(o, 1500)) woken++; } }); t.Start(); all.Add(t); }\nentered.Wait();\nfor (var k = 0; k < 2; k++) { lock (o) { System.Threading.Thread.Sleep(70); System.Threading.Monitor.Pulse(o); } System.Threading.Thread.Sleep(30); }\nforeach (var t in all) t.Join();\nwoken", "2")] + [InlineData("try { System.Threading.Monitor.Pulse(new object()); } catch (System.Threading.SynchronizationLockException) { return \"sync\"; }\nreturn \"\";", "sync")] [InlineData("var e = new System.Threading.ManualResetEventSlim();\nTask.Run(() => { System.Threading.Thread.Sleep(150); e.Set(); });\ne.Wait();\ne.IsSet", "True")] [InlineData("Task.Run(async () => { await Task.Delay(150); return 4; }).GetAwaiter().GetResult()", "4")] [InlineData("try { Task.Run(() => { System.Threading.Thread.Sleep(100); throw new InvalidOperationException(\"r\"); return 1; }).GetAwaiter().GetResult(); } catch (InvalidOperationException e) { return e.Message; }\nreturn \"\";", "r")] diff --git a/dotnet/CodeExec/ExecutionGuard.cs b/dotnet/CodeExec/ExecutionGuard.cs index 37cbc9b..59ef692 100644 --- a/dotnet/CodeExec/ExecutionGuard.cs +++ b/dotnet/CodeExec/ExecutionGuard.cs @@ -80,7 +80,7 @@ public static class ExecutionGuard /// 10 ms apart, back to every iteration when they are not: slow iterations overshoot the budget by about 20 ms, /// fast ones pay one clock read per 256 iterations. The statics are shared by all threads: a race only shifts a /// check. Work handed to a thread or task start (, ) carries its run: a - /// thread-static while it runs synchronously, an AsyncLocal for asynchronous task work (it flows into the + /// thread-local while it runs synchronously, an AsyncLocal for asynchronous task work (it flows into the /// work's awaits); the tasks and threads it starts in turn belong to the same run. Once a run that started work has /// ended, every clock check reads it, and work of an ended run stops there. A thread entry swallows the stop: an /// exception left unhandled on a thread would end the process. @@ -94,7 +94,11 @@ internal static class __CodeExecGuard static Run _current; // The run of the work running: synchronous work on this thread, and asynchronous task work (it flows into the work's // awaits). Nothing else carries it: a timer, an async void continuation or a thread started by work keeps none of it. - [global::System.ThreadStatic] static Run _thread; + // A ThreadLocal over object, not a thread-static: this assembly is collectible, and with the thread-static .NET 10 + // test runs crashed now and then in the GC (a null method table while it scanned for finalization). The suspect is + // the runtime's thread-static storage of an unloaded assembly on threads that outlive it; ThreadLocal keeps + // its storage in the framework (ThreadLocal would have thread-statics of a collectible instantiation again). + static readonly global::System.Threading.ThreadLocal _thread = new global::System.Threading.ThreadLocal(); static readonly global::System.Threading.AsyncLocal _async = new global::System.Threading.AsyncLocal(); /// One run: whether it has ended, whether it started work, and how much of that work has not finished. @@ -141,7 +145,8 @@ public static bool Init() public static bool Running() => _stopped == 0 && (_orphans == 0 || (_live == 0 && _asyncOrphans == 0) || !Orphaned()); // The AsyncLocal is slow to read: only once asynchronous task work has ever run. - static Run Current() => _thread ?? (_asyncWork != 0 ? _async.Value : null); + // IsValueCreated first: reading Value on a thread that never set it would allocate its storage there. + static Run Current() => (_thread.IsValueCreated ? (Run)_thread.Value : null) ?? (_asyncWork != 0 ? _async.Value : null); static bool Orphaned() { @@ -184,16 +189,16 @@ static void Done(Run run) static void Do(Run run, global::System.Action work, bool swallow) { Started(); - var thread = _thread; + var thread = _thread.IsValueCreated ? _thread.Value : null; var flowing = _asyncWork != 0 ? _async.Value : null; - _thread = run; + _thread.Value = run; if (flowing != null) _async.Value = null; // what the work starts without a wrapper (a timer, an async void continuation) is not the run's try { work(); } catch (global::System.Exception) when (swallow && !Running()) { } finally { - _thread = thread; + _thread.Value = thread; if (flowing != null) _async.Value = flowing; Done(run); @@ -214,9 +219,9 @@ static TResult Get(Run run, global::System.Func work) if (global::System.Threading.Volatile.Read(ref run.Ended) != 0) _asyncOrphans = 1; Started(); - var thread = _thread; + var thread = _thread.IsValueCreated ? _thread.Value : null; var flowing = _async.Value; - _thread = null; + _thread.Value = null; _async.Value = run; try { @@ -235,7 +240,7 @@ static TResult Get(Run run, global::System.Func work) } finally { - _thread = thread; + _thread.Value = thread; _async.Value = flowing; } } @@ -281,8 +286,7 @@ static bool Sliced(int timeout, global::System.Func wait) { if (timeout > 0) { - var elapsed = (global::System.Diagnostics.Stopwatch.GetTimestamp() - start) * 1000 / global::System.Diagnostics.Stopwatch.Frequency; - var left = timeout - elapsed; + var left = timeout - ElapsedMs(start); if (left <= SliceMs) return wait(left > 0 ? (int)left : 0); } @@ -292,6 +296,9 @@ static bool Sliced(int timeout, global::System.Func wait) } } + static long ElapsedMs(long start) => + (global::System.Diagnostics.Stopwatch.GetTimestamp() - start) * 1000 / global::System.Diagnostics.Stopwatch.Frequency; + /// A TimeSpan timeout in ms; -2 when out of range (the framework's own wait throws for it). static int Ms(global::System.TimeSpan timeout) { @@ -406,10 +413,80 @@ public static T Ready(T task) where T : global::System.Threading.Tasks.Task public static bool CountdownWait(global::System.Threading.CountdownEvent waitable, global::System.TimeSpan timeout, global::System.Threading.CancellationToken cancellationToken) => CountdownWait(waitable, Ms(timeout), cancellationToken); public static bool CountdownWait(global::System.Threading.CountdownEvent waitable, int millisecondsTimeout, global::System.Threading.CancellationToken cancellationToken) => Sliced(millisecondsTimeout, ms => waitable.Wait(ms, cancellationToken)); - // Waiting again after a slice misses no Pulse: the lock is held again between two slices. - public static bool MonitorWait(object obj) => Sliced(-1, ms => global::System.Threading.Monitor.Wait(obj, ms)); - public static bool MonitorWait(object obj, int millisecondsTimeout) => Sliced(millisecondsTimeout, ms => global::System.Threading.Monitor.Wait(obj, ms)); - public static bool MonitorWait(object obj, global::System.TimeSpan timeout) => Sliced(Ms(timeout), ms => global::System.Threading.Monitor.Wait(obj, ms)); + // A slice that times out leaves the wait queue until it holds the lock again: a Pulse in that gap would be lost (the + // pulsing thread may take the lock first). So the agent's Pulse / PulseAll also count the wake-ups they owe the + // object's sliced waiters (under its lock), and only that count wakes one: a waiter woken by the Pulse, or one between + // two slices, takes it once it holds the lock again; whoever finds none waits on. One Pulse wakes one waiter. + sealed class Waiting + { + public int Waiters, Owed; + } + + static readonly global::System.Runtime.CompilerServices.ConditionalWeakTable _waiting = + new global::System.Runtime.CompilerServices.ConditionalWeakTable(); + + static Waiting WaitingOn(object obj) + { + Waiting waiting; + return obj != null && _waiting.TryGetValue(obj, out waiting) ? waiting : null; + } + + public static bool MonitorWait(object obj) => MonitorWait(obj, -1); + public static bool MonitorWait(object obj, global::System.TimeSpan timeout) => MonitorWait(obj, Ms(timeout)); + public static bool MonitorWait(object obj, int millisecondsTimeout) + { + if (obj == null || millisecondsTimeout < -1 || !global::System.Threading.Monitor.IsEntered(obj)) + return global::System.Threading.Monitor.Wait(obj, millisecondsTimeout); // the framework's own exception + var waiting = _waiting.GetValue(obj, _ => new Waiting()); + waiting.Waiters++; + try + { + return Sliced(millisecondsTimeout, ms => TakeWakeUp(obj, waiting, ms)); + } + finally + { + // Whatever ends the wait, the lock is held again here. + if (--waiting.Waiters == 0) + _waiting.Remove(obj); // no entry outlives the wait: it would keep the snippet's code loaded + else if (waiting.Owed > waiting.Waiters) + waiting.Owed = waiting.Waiters; + } + } + + // Waits up to ms (-1: without bound) for a wake-up owed to the object's waiters. Waking without one (it went to + // another waiter) waits on for the time left, so a short or last slice does not end early. + static bool TakeWakeUp(object obj, Waiting waiting, int ms) + { + var start = global::System.Diagnostics.Stopwatch.GetTimestamp(); + for (;;) + { + var left = ms <= 0 ? ms : (int)global::System.Math.Max(0, ms - ElapsedMs(start)); + global::System.Threading.Monitor.Wait(obj, left); + if (waiting.Owed > 0) + { + waiting.Owed--; + return true; + } + if (left == 0) + return false; + } + } + + public static void MonitorPulse(object obj) + { + global::System.Threading.Monitor.Pulse(obj); // throws, as written, when this thread does not hold the lock + var waiting = WaitingOn(obj); + if (waiting != null && waiting.Owed < waiting.Waiters) + waiting.Owed++; + } + + public static void MonitorPulseAll(object obj) + { + global::System.Threading.Monitor.PulseAll(obj); + var waiting = WaitingOn(obj); + if (waiting != null) + waiting.Owed = waiting.Waiters; + } public static bool WaitOne(global::System.Threading.WaitHandle handle) => Sliced(-1, ms => handle.WaitOne(ms)); public static bool WaitOne(global::System.Threading.WaitHandle handle, int millisecondsTimeout) => Sliced(millisecondsTimeout, ms => handle.WaitOne(ms)); diff --git a/spec/contract.md b/spec/contract.md index 1570977..8ce15d9 100644 --- a/spec/contract.md +++ b/spec/contract.md @@ -164,7 +164,10 @@ the host turns them off (`CodeRunnerOptions.ExecutionGuards`): context's thread runs there and returns its value. A task the scheduler declines, one with no body to run (an async method's, whose continuation waits for the blocked thread), a wait on a thread-pool thread, and every wait on a runtime without the framework's inline path are waited for in slices and stop at the budget. A body run inline that - is not the snippet's own code is stopped only when it returns. Not covered (stopped + is not the snippet's own code is stopped only when it returns. `Monitor.Pulse` / `PulseAll` in the agent's code become + stand-ins too: a pulse sent while a sliced `Monitor.Wait` is between two slices still wakes it, and one `Pulse` wakes + one waiter (a pulse sent from code the guards do not rewrite can still be missed by a waiter between two slices). + Not covered (stopped only when the call returns): such a wait with named arguments, inside a conditional access (`a?.Task.Result`), as a property pattern path (`x is { Task.Result: 1 }`), through `base.`, a method group or a delegate; .NET 9+ `Task.WaitAll` given a span; `lock` / `Monitor.Enter` and other waits (`BlockingCollection.Take`, `Barrier`, @@ -301,6 +304,8 @@ Lengths are in characters: UTF-16 code units in C#, code points in Python. A lim ## Changelog +- **Unreleased** — C#: a `Monitor.Pulse` sent while a sliced `Monitor.Wait` is between two slices wakes it (it was + lost, and the waiter slept until the budget); the agent's `Monitor.Pulse` / `PulseAll` become guard stand-ins (§3.6). - **v0.4.0** — execution guards (§3.6): C# snippets, full source and includes stop with `Runtime` (`Recursion too deep`) before a stack overflow and with `Timeout` once `timeoutMs` is spent, also in synchronous code on the main thread; Python stops with `Timeout` through a watchdog (§4). `timeoutMs` applies to both runtimes; `0`