RewriteRecording holds its ambient scope in a [ThreadStatic] field:
public sealed class RewriteRecording : IDisposable
{
[ThreadStatic]
private static RewriteRecording? current;
That is the same shape MathS.Settings had before #857, and it has the same two consequences:
- a recording opened before an
await is not in force after it — the continuation resumes on a pool thread that has never seen it, so the steps go uncollected;
- a thread returned to the pool still carries whatever recording was left on it, so the next caller to borrow that thread can have its steps quietly added to somebody else's list.
Neither surfaces as an error. The first loses output, the second mixes two callers' output together.
Why this is filed separately
It was deliberately left out of #857. Its documentation is explicit about the current semantics —
Opens a recording on this thread.
— so unlike the settings it is not promising something it fails to deliver. But after #857 it is the only ambient user-facing scope in the library that is still per-thread, and "on this thread" is a surprising contract for a scope held in a using around code that may well await.
Suggested fix
The same one: AsyncLocal, which MathS.Multithreading already used for the cancellation token and which Setting<T> now uses.
Note the trap #857 ran into — AsyncLocal<RewriteRecording> on the static field is not sufficient by itself. An AsyncLocal flows the reference, so if the recording is mutated in place, two concurrent flows sharing one instance will write into the same step list. RewriteRecording accumulates into a List<RewriteStep>, so this applies directly. What must be per-flow is the current recording pointer; each recording instance should belong to exactly one flow, with nesting handled by the existing enclosing chain.
Explicitly not in scope
The recursion-depth counters and per-thread scratch caches (Gruntz.depth, lHopitalDepth, FastExpression.scratch, the constant caches) should keep [ThreadStatic]. A recursion depth must not follow a call into a sibling task.
RewriteRecordingholds its ambient scope in a[ThreadStatic]field:That is the same shape
MathS.Settingshad before #857, and it has the same two consequences:awaitis not in force after it — the continuation resumes on a pool thread that has never seen it, so the steps go uncollected;Neither surfaces as an error. The first loses output, the second mixes two callers' output together.
Why this is filed separately
It was deliberately left out of #857. Its documentation is explicit about the current semantics —
— so unlike the settings it is not promising something it fails to deliver. But after #857 it is the only ambient user-facing scope in the library that is still per-thread, and "on this thread" is a surprising contract for a scope held in a
usingaround code that may well await.Suggested fix
The same one:
AsyncLocal, whichMathS.Multithreadingalready used for the cancellation token and whichSetting<T>now uses.Note the trap #857 ran into —
AsyncLocal<RewriteRecording>on the static field is not sufficient by itself. AnAsyncLocalflows the reference, so if the recording is mutated in place, two concurrent flows sharing one instance will write into the same step list.RewriteRecordingaccumulates into aList<RewriteStep>, so this applies directly. What must be per-flow is the current recording pointer; each recording instance should belong to exactly one flow, with nesting handled by the existingenclosingchain.Explicitly not in scope
The recursion-depth counters and per-thread scratch caches (
Gruntz.depth,lHopitalDepth,FastExpression.scratch, the constant caches) should keep[ThreadStatic]. A recursion depth must not follow a call into a sibling task.