Skip to content

Ensure a delegate built for the old tracing mode doesn't replace the new one - #2118

Open
BCSharp wants to merge 1 commit into
IronLanguages:mainfrom
BCSharp:trace-bkgnd
Open

BCSharp wants to merge 1 commit into
IronLanguages:mainfrom
BCSharp:trace-bkgnd

Conversation

@BCSharp

@BCSharp BCSharp commented Oct 24, 2026

Copy link
Copy Markdown
Member

The bug: switching tracing on or off (sys.settrace) updates the target of every code object, but other writers of the target are not synchronized with that switch: the adaptive compiler's completion callbacks, the background precompile of class bodies, compilation on first use in MakeClass and FunctionCode.Call, and -X:Debug function definitions.
A delegate built for the previous mode could thus become the target after the switch and stay until the next one, e.g. a class body that runs untraced while tracing is on.

These writers now set the target through FunctionCode.PublishTarget, which skips a delegate built for a mode that is no longer current (it stays in its cache for when the mode switches back) and checks the mode again after setting the target, repeating the switch's update if the mode changed meanwhile. SetTrace changes the mode with an interlocked operation, so the new mode is visible before any code is updated for it.

Under -X:Debug this also fixes functions defined while tracing is on, which always got their normal delegate and were not traced.

@slozier slozier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks!

SetTarget(target);
Thread.MemoryBarrier();
while (context.EnableTracing != tracing) {
tracing = context.EnableTracing;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of reading context.EnableTracing (and another one at the top of UpdateDelegate). Any chance of things getting out of sync (seems unlikely but doesn't hurt to ask).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants