The attached script looks innocent, but gives wildly incorrect results on all versions of CPython I've tested.

It does two things:

• spawns a thread which just loops, doing nothing

• in the main thread, repeatedly increments a variable 'x'

And most of the increments of the variable are lost!

This requires two key things I haven't mentioned, but that you wouldn't expect to change anything. First, the thread function closes over the local variable 'x' (though it doesn't touch this variable in any way). And second, the thread function has a Python-level trace function registered (but this trace function is a no-op).

Here's what's going on:

When you use sys.settrace() to install a Python-level trace function, it installs the C-level trace function "call_trampoline". And then whenever a trace event happens, call_trampoline calls PyFrame_FastToLocalsWithError, then calls the Python-level trace function, then calls PyFrame_LocalsToFast (see: https://github.com/python/cpython/blob/master/Python/sysmodule.c#L384-L395). This save/call/restore sequence is safe if all the variables being saved/restored are only visible to the current thread, which used to be true back in the days when local variables were really local. But it turns out nowadays (since, uh... python 2.1), local variables can be closed over, and thus can visible from multiple threads simultaneously.

Which means we get the following sequence of events:

• In thread A, a trace event occurs (e.g., executing a line of code)

• In thread A, call_trampoline does PyFrame_FastToLocalsWithError, which saves a snapshot of the current value of 'x'

• In thread A, call_trampoline then starts calling the trace function

• In thread B, we increment 'x'

• In thread A, the trace function returns

• In thread A, call_trampoline then does PyFrame_LocalsToFast, which restores the old value of 'x', overwriting thread B's modifications

This means that merely installing a Python-level trace function – for example, by running with 'python -m trace' or under pdb – can cause otherwise correct code to give wrong answers, in an incredibly obscure fashion.

In real life, I originally ran into this in a substantially more complicated situation involving PyPy and coverage.py and a nonlocal variable that was protected by a mutex, which you can read about here: https://bitbucket.org/pypy/pypy/issues/2591/
It turns out that since this only affects *Python*-level trace functions, merely recording coverage information using coverage.py doesn't normally trigger it, since on CPython coverage.py tries to install an accelerator module that uses a *C*-level trace function. Which is lucky for my particular case. But probably it should be fixed anyway :-).

CC'ing belopolsky as the interest area for the trace module, Mark Shannon because he seems to enjoy really pathological low-level bugs, and Benjamin and Yury as the "bytecode" interest area. I'm surprised that there's apparently no interest area for, like, "variables should retain the values they are assigned" -- apologies if I've CC'ed anyone incorrectly.

A version of the same problem without threads, using generators instead to get the bug deterministically. Prints 1, 1, 1, 1 on CPython and 1, 2, 3, 3 on PyPy; in both cases we would rather expect 1, 2, 3, 4.

(Note: x.py is for Python 2.7; for 3.x, of course, replace .next() with .__next__(). The result is the same)

Some thoughts based on discussion with Armin in #pypy:

It turns out if you simply delete the LocalsToFast and FastToLocals calls in call_trampoline, then the test suite still passes. I'm pretty sure that pdb relies on this as a way to set local variables, though, so I think this is probably more of a bug in the test suite than anything else.

In some ways, it seems like the most attractive fix to this would be to have the Python-level locals() and frame.f_locals return a dict proxy object whose __setitem__ writes any mutations back to both the fast array and the real frame->f_locals object, so that LocalsToFast becomes unnecessary. As Armin points out, though, that would definitely be a semantic change: there may be code out there that relies on locals() returning a dict – technically it doesn't have to return a dict even now, but it almost always does – or that assumes it can mutate the locals() array without that affecting the function. I think this would arguably be a *good* thing; it would make the behavior of locals() and f_locals much less confusing in general. But he's right that it's something we can only consider for 3.7.

The only more-compatible version I can think of is: before calling the trace function, do FastToLocals, and also make a "shadow copy" of all the cellvars and freevars. Then after the trace function returns, when copying back from LocalsToFast, check against these shadow copies, and only write back cellvars/freevars that the trace function actually modified (where by modified we mean 'old is not new', just a pointer comparison). Since most functions have no cellvars or freevars, and since we would only need to do this when there's a Python-level trace function active, this shouldn't add much overhead. And it should be compatible enough to backport even to 2.7, I think.

The writeback-only-if-changed approach sounds like a plausible improvement to me. I'd be hesitant to include such a change in 3.5.4 though, since we don't get a second go at that if something breaks unexpectedly.

However, I don't think returning a write-through proxy from locals() would be a good idea, since you can still have race conditions between "read-update-writeback" operations that affect the cells directly, as well as with those that use the new write-through proxy. At the moment, you can only get yourself into trouble by way of operations that have access to LocalsToFast, and those are pretty rare now that exec is no longer a statement.

If folks really want that write-through behaviour (at least for closure variables), 3.7 will let them write it themselves, since closure cells are becoming writeable from pure Python code:

>>> def outer():
...     x = 1
...     def inner():
...         return x
...     return inner
... 
>>> f = outer()
>>> f.__closure__[0].cell_contents
1
>>> f.__closure__[0].cell_contents = 2
>>> f()
2
>>> f.__closure__[0].cell_contents = "hello"
>>> f()
'hello'

It isn't obvious to me whether the write-through proxy idea is a good one on net, but here's the rationale for why it might be.

Currently, the user-visible semantics of locals() and f_locals are a bit complicated. AFAIK they aren't documented anywhere outside the CPython and PyPy source (PyPy is careful to match all these details), but let me take a stab at it:

The mapping object you get from locals()/f_locals represents the relevant frame's local namespace. For many frames (e.g. module level, class level, anything at the REPL), it literally *is* the local namespace: changes made by executing bytecode are immediately represented in it, and changes made to it are immediately visible to executing bytecode. Except, for function frames, it acts more like a snapshot copy of the local namespace: it shows you the namespace at the moment you call locals(), but then future changes to either the code's namespace or the object don't affect each other. Except except, the snapshot might be automatically updated later to incorporate namespace changes, e.g. if I do 'ns = locals()' and then later on someone accesses the frame's f_locals attribute, then reading that attribute will cause my 'ns' object to be silently updated. But it's still a snapshot; modifications to the mapping aren't visible to the executing frame. Except**3, if you happen to modify the mapping object while you're inside a trace function callback, then *those* modifications are visible to the executing frame. (And also if a function is being traced then as a side-effect this means that now our 'ns' object above does stay constantly up to date.) Except**4, you don't actually have to be inside a trace function callback for your modifications to be visible to the executing frame – all that's necessary is that *some* thread somewhere is currently inside a trace callback (even if it doesn't modify or even look at the locals itself, as e.g. coverage.py doesn't).

This causes a lot of confusion [1].

On top of that, we have this bug here. The writeback-only-if-changed idea would make it so that we at least correctly implement the semantics I described in the long paragraph above. But I wonder if maybe we should consider this an opportunity to fix the underlying problem, which is that allowing skew between locals() and the actual execution namespace is this ongoing factory for bugs and despair. Specifically, I'm wondering if we could make the semantics be:

"locals() and f_locals return a dict-like object representing the local namespace of the given frame. Modifying this object and modifying the corresponding local variables are equivalent operations in all cases."

(So I guess this would mean a proxy object that on reads checks the fast array first and then falls back to the dict, and on writes updates the fast array as well as the dict.)

you can still have race conditions between "read-update-writeback" operations that affect the cells directly, as well as with those that use the new write-through proxy.

Sure, but that's just a standard concurrent access problem, no different from any other case where you have two different threads trying to mutate the same local variable or dictionary key at the same time. Everyone who uses threads knows that if you want to do that then you need a mutex, and if you don't use proper locking then it's widely understood how to recognize and debug the resulting failure modes. OTOH, the current situation where modifications to the locals object sometimes affect the namespace, and sometimes not, and sometimes they get overwritten, and sometimes they don't, and it sometimes depends on spooky unrelated things like "is some other thread currently being traced"? That's *way* more confusing that figuring out that there might be a race condition between 'x = 1' and 'locals()["x"] = 2'.

Plus, pdb depends on write-through working, and there are lots of frames that don't even use the fast array and already have my proposed semantics. So realistically our choices are either "consistently write-through" or "inconsistently write-through".

[1] https://www.google.com/search?q=python+modify+locals&ie=utf-8&oe=utf-8

To make the behaviour more consistent in 3.7, I'd be more inclined to go in the other direction: make locals() return a truly independent snapshot when used in a function, rather than sharing a single snapshot between all locals() calls.

Shared snapshots that may potentially be written back to the frame locals would then be unique to trace functions, rather than being a feature of function level locals() calls in general.

Interesting idea! I'm not sure I fully understand how it would work though.

What would you do for the frames that don't use the fast array, and where locals() currently returns the "real" namespace?

How are you imagining that the trace function writeback would be implemented? Some sort of thread-local flag saying "we're inside a trace function for frame XX" that causes locals() and f_locals to switch to returning a "real" namespace object?

Sorry, I wasn't clear: I don't see any problem for the cases that don't optimize local variable access, and don't think any of those should change.

Instead, I think we should tighten up the formal specification of locals() to better match how it is actually used in practice: https://mail.python.org/pipermail/python-dev/2013-May/125917.html

(https://bugs.python.org/issue17960 is the corresponding issue, although I clearly got distracted by other things and never followed up with a patch for the language reference. https://bugs.python.org/issue17546 is another issue lamenting the current underspecification in this area)

However, function bodiess are already inherently different from other execution namespaces, and that stems from a particular special case assumption that we don't make anywhere else: we assume that at compile time, the compiler can see all of the names added to the local namespace of a function.

That assumption wasn't quite valid in Python 2 (since unqualified exec statements and function level wildcard imports could mess with it), but it's much closer to being true in Python 3.

Checking the 3.7 code, the only remaining ways to trigger it are:

• via a tracing function (since LocalsToFast gets called after the tracing function runs)
• by injecting an IMPORT_STAR opcode into a function code object (the compiler disallows that in Python 3 and emits a SyntaxWarning for it in Python 2, but the LocalsToFast call is still there in the eval loop)

So I think an entirely valid way forward here would be to delete LocalsToFast in 3.7+, and say that if you want to write access to a function namespace from outside the function, you need to either implement an eval hook (not just a tracing hook), or else use a closure that closes over all the variables that you want write access to.

However, the less drastic way forward would be to make it so that writing a tracing function is the only way to get access to the FastToLocals result, and have locals() on a frame running a code object compiled for fast locals return f->f_locals.copy() rather than a direct reference to the original.

I updated the old "we should clarify the semantics" issue with a more concrete update proposal: https://bugs.python.org/issue17960#msg296880

Essentially nothing would change for module and class scopes, but the proposal for function scopes is that locals() be changed to return "frame.f_locals.copy()" rather than a direct reference to the original.

Nothing would change for tracing functions (since they already access frame.f_locals directly), but the current odd side-effect that setting a trace function has on the result of normal locals() calls would go away.

Folks that actually *wanted* the old behaviour would then need to do either "sys._getframe().f_locals" or "inspect.currentframe().f_locals".

However, none of that would help with *this* issue: resolving the bug here would still require either a modification that allowed PyFrame_LocalsToFast to only write back those values that had been rebound since the preceding call to PyFrame_FastToLocals (ma_version_tag could help with doing that efficiently), or else a decision to disallow write-backs to frame locals even from tracing functions in 3.7+.

Folks that actually *wanted* the old behaviour would then need to do either "sys._getframe().f_locals" or "inspect.currentframe().f_locals".

So by making locals() and f_locals have different semantics, we'd be adding yet another user-visible special-case? That seems unfortunate to me.

if you want to write access to a function namespace from outside the function, you need to either implement an eval hook (not just a tracing hook)
[...]
or else a decision to disallow write-backs to frame locals even from tracing functions in 3.7+.

Disallowing writeback from tracing functions would completely break bdb/pdb, so unless you're planning to rewrite bdb in C as an eval hook, then I don't think this is going to happen :-). Writing back to locals is a useful and important feature!

I think I'm missing some rationale here for why you prefer this approach – it seems much more complicated in terms of user-visible semantics, and possibly implementation-wise as well.

I like it because it categorically eliminates the "tracing or not?" global state dependence when it comes to manipulation of the return value of locals() - manipulating that will either always affect the original execution namespace immediately (modules, classes, exec, eval), or always be a completely independent snapshot that can never lead to changes in the original name bindings (functions, generators, coroutines).

As a result, the locals() documentation updates for issue bpo-17960 wouldn't need to mention trace functions at all.

Instead, the only folks that would need to worry about potentially unexpected updates to the internal state of functions, generators, and coroutines when tracing is in effect would be those accessing frame.f_locals directly. That state dependence can then be documented specifically as part of the f_locals documentation, and users of that attribute can make their own copy if they want to ensure that their subsequent mutations definitely can't affect the original namespace, even when tracing is in effect.

I like it because it categorically eliminates the "tracing or not?" global state dependence when it comes to manipulation of the return value of locals() - manipulating that will either always affect the original execution namespace immediately (modules, classes, exec, eval), or always be a completely independent snapshot that can never lead to changes in the original name bindings (functions, generators, coroutines).

Maybe I was unclear...? my question is why you prefer locals-making-a-copy over locals-and-f_locals-for-function-frames-return-proxy-objects. It seems to me that both of these proposals eliminate the "tracing or not?" global state dependence (right?), so this can't be a reason to prefer one over the other. And the latter additionally eliminates the distinction between modules/classes/exec/eval and functions/generators/coroutines, while also avoiding introducing a distinction between locals() and f_locals.

The problem I see with proxy objects for functions/coroutines/generators is that it *doesn't* match how locals() currently behaves in the absence of a tracing function - that gives you a "single shared snapshot" behaviour, where writes to the result of locals() *don't* affect the original namespace.

I agree that replacing frame.f_locals with a write-through proxy would be a good way to get rid of PyFrame_LocalsToFast, though (and thus fix the bug this issue covers).

The point where we disagree is that I think we should replace the tracing-or-not distinction with a locals()-or-frame.f_locals distinction, not get rid of the distinction entirely.

In msg296758 Nathaniel wrote:

It turns out if you simply delete the LocalsToFast and FastToLocals calls in call_trampoline, then the test suite still passes. I'm pretty sure that pdb relies on this as a way to set local variables, though, so I think this is probably more of a bug in the test suite than anything else.

Maybe a point to be taken in this issue is that pdb does a poor usage of this feature as shown in bpo-9633: changing the value of a local only works in limited cases.

As per the discussion in issue bpo-17960, I'm going to put together a short PEP about this topic for Python 3.7 that will better specify the expected behaviour of locals() and frame.f_locals.

That will be a combination of making the status quo official, proposing some changes, and asking Guido for a design decision.

Documenting the status quo: make the current behaviour at module scope, class scope, and the corresponding behaviours of exec and eval officially part of the status quo

Proposing a change: replacing the current LocalsToFast approach with a custom write-through proxy type that updates the locals array and nonlocal cell variables immediately whenever frame.f_locals is modified (this is the part that should fix the bug reported in this issue)

Requesting a design decision: whether locals() at function(/generator/coroutine) scope should return:

• a direct reference to the write-through proxy
• a live read-only dict-proxy (ala class __dict__ attributes)
• a point-in-time snapshot (i.e. copying the namespace)

Err, s/officially part of the status quo/officially part of the language specification/ :)

After drafting PEP-558, briefly chatting about it to Guido, and sleeping on the topic, I'm wondering if there might be answer that's simpler than any of the alternatives consider so far: what if PyFrame_FastToLocals added the *cell objects* to f_locals for any variables that the compiler would access using LOAD/STORE_DEREF (rather than LOAD/STORE_FAST), and then PyFrame_LocalsToFast only wrote back the entries for variables that used LOAD/STORE_FAST?

That would be enough to fix the reported problems (since PyFrame_LocalsToFast would always leave closure variables alone in both the function defining the closure and the ones referencing it), and a trace hook that actually *wanted* to update the closure references could write to the "cell_contents" attribute on the cell of interest.

How does a trace function or debugger tell the difference between a closure cell and a fast local whose value happens to be a cell object?

The same way the dis module does: by looking at the names listed in the various code object attributes.

If it's listed in co_cellvars, then it's a local variable in the current frame that's in a cell because it's part of the closure for a nested function.

If it's listed in co_freevars, then it's a nonlocal closure reference.

Otherwise, it's a regular local variable that just happens to be holding a reference to a cell object.

So if all we did was to put the cell objects in the frame.f_locals dict, then trace functions that supported setting attributes (including pdb) would need to be updated to be cell aware:

    def setlocal(frame, name, value):
        if name in frame.f_code.co_cellvars or name in frame.f_code.co_freevars:
            frame.f_locals[name].cell_contents = value
        else:
            frame.f_locals[name] = value

However, to make this more backwards compatible, we could also make it so that *if* a cell entry was replaced with a different object, then PyFrame_LocalsToFast would write that replacement object back into the cell.

Even with this more constrained change to the semantics frame.f_locals at function level, we'd probably still want to keep the old locals() semantics for the builtin itself - that has lots of string formatting and other use cases where having cell objects suddenly start turning up as values would be surprising.

https://github.com/python/cpython/pull/3640/files includes a more automated-tested-friendly version of Armin's deterministic reproducer (the generator loop uses range() to generator a separate loop counter, and then the test checks that the incremented closure variable matches that loop counter), together with a fix based on the idea of putting actual cell objects into frame.f_locals while a trace hook is running. It turned out a lot of the necessary machinery was already there, since CPython already uses a common pair of utility functions (map_to_dict/dict_to_map) to handle fast locals, cell variables, and nonlocal variables.

PEP-558 still needs another editing pass before I send it to python-dev, and the PR needs some additional test cases to explicitly cover the expected locals() and frame.f_locals semantics at different scopes when a Python-level trace hook is installed, but I'm happy now that this is a reasonable and minimalistic way to resolve the original problem.

Doesn't this proposal break every debugger, including pdb?

For write-backs: no, since the interpreter will still write those values back into the destination cell

For locals display: no, since nothing changes for the handling of fast locals

For closure display: yes as, by default, debuggers will now print the closure cell, not the value the cell references - they'd need to be updated to display obj.cell_contents for items listed in co_freevars and co_cellvars.

That's why PEP-588 needs to be a PEP - there's a language design question around the trade-off between requiring all future Python implementations to implement a new write-through proxy type solely for this use case, or instead requiring trace functions to cope with co_freevars and co_cellvars being passed through as cell objects, rather than as direct references to their values.

I've been thinking further about the write-through proxy idea, and I think I've come up with a design for one that shouldn't be too hard to implement, while also avoiding all of the problems that we want to avoid.

The core of the idea is that the proxy type would just be a wrapper around two dictionaries:

• the existing f_locals dictionary
• a new dictionary mapping cell & free variable names to their respective cells (note: this may not actually need to be a dict, as a direct reference from the proxy back to the frame may also suffice. However, I find it easier to think about the design by assuming this will be a lazily initialised dict in its own right)

Most operations on the proxy would just be passed through to f_locals, but for keys in both dictionaries, set and delete operations would *also* affect the cell in the cell dictionary. (Fortunately dict views don't expose any mutation methods, or intercepting all changes to the mapping would be a lot trickier)

Frames would gain a new lazily initialised "f_traceproxy" field that defaults to NULL/None.

For code objects that don't define or reference any cells, nothing would change relative to today.

For code objects that *do* define or reference cells though, tracing would change as follows:

• before calling the trace function:
  • f_locals would be updated from the fast locals array and current cell values as usual
  • f_locals on the frame would be swapped out for f_traceproxy (creating the latter if needed)
• after returning from the trace function:
  • f_locals on the frame would be reset back to bypassing the proxy (so writes to f_locals stop being written through to cells when the trace hook isn't running)
  • only the actual locals would be written from f_locals back to the fast locals array (cell updates are assumed to have already been handled via the proxy)

This preserves all current behaviour *except* the unwanted one of resetting cells back to their pre-tracehook value after returning from a trace hook:

• code outside trace hooks can't mutate the function level fast locals or cells via locals() or frame.f_locals (since their modifications will be overwritten immediately before the trace function runs), but *can* treat it as a regular namespace otherwise
• code inside trace hooks can mutate function level fast locals and cells just by modifying frame.f_locals
• all code can display the current value of function level fast locals and cells just by displaying locals() or frame.f_locals
• there's still only one f_locals dictionary per frame, it may just have a proxy object intercepting writes to cell variables when a trace hook is running

That way, we can avoid the problem with overwriting cells back to old values, *without* enabling arbitrary writes to function locals from outside trace functions, and without introducing any tricky new state synchronisation problems.

Wow, nothing here is simple. :-( Even though the examples show that there's obviously a bug, I would caution against fixing it rashly without understanding how the current behavior may be useful in other circumstances. I've lost what I recall from reading PEP-558 so I can't quite fathom the new design, but I wish you good luck trying to implement it, and hopefully it will work out as hoped for (simpler, solving the issue, keeping the useful behaviors).

Aye, what's in PEP-558 is the least invasive implementation change I've been able to come up that fixes the reported bug, but Nathaniel's right that it would break debuggers (and probably some other things) as currently written.

So the above design comes from asking myself "How can I get the *effect* of PEP-558, while hiding what's going on internally even from trace function implementations?".

While I can't be sure until I've actually implemented it successfully (no ETA on that, unfortunately), I think blending my idea with Nathaniel's original idea will let us enjoy the benefits of both approaches, without the downsides of either of them.

I guess I should say that I'm still confused about why we're coming up with such elaborate schemes here, instead of declaring that f_locals and locals() shall return a dict proxy so that from the user's point of view, they Always Just Work The Way Everyone Expects.

The arguments against that proposal I'm aware of are:

1. Implementing a full dict-like mapping object in C is tiresome. But your proposal above also requires doing this, so presumably that's not an issue.

2. We want to keep the current super-complicated and confusing locals() semantics, because we like making life difficult for alternative implementations (PyPy at least exactly copies all the weird details of how CPython's locals() works, which is why it inherited this bug), and by making the language more confusing we can encourage the use of linters and boost Python's Stackoverflow stats. ...I guess my bias against this argument is showing :-). But seriously, if we want to discourage writing to locals() then the way to do that is to formally deprecate it, not go out of our way to make it silently unreliable.

3. According to the language spec, all Python implementations have to support locals(), but only some of them have to support frame introspection, f_locals, debugging, and mutation of locals. But... I think this is a place where the language spec is out of touch with reality. I did a quick survey and AFAICT in practice, Python implementations either support *both* locals() and f_locals (CPython, PyPy, Jython, IronPython), or else they support *neither* locals() nor f_locals (MicroPython -- in fact MicroPython defines locals() to unconditionally return an empty dict). We could certainly document that supporting writes through locals() is a quality-of-implementation thing CPython provides, similar to the prompt destruction guarantees provided by refcounting. But I don't think implementing this is much of a burden -- if you have enough introspection metadata to get the list of locals and figure out where their values are stored in memory (which is the absolute minimum to implement locals()), then you probably also have enough metadata to write back to those same locations. Plus debugger support is obviously a priority for any serious full-fledged implementation.

So the original write-through proxy idea still seems like the best solution to me.

FWIW, a Psyco-level JIT compiler can support reads from locals() or f_locals, but writes are harder. The need to support writes would likely become another hard step on the way towards adding some simple JIT support to CPython in the future, should you decide you ever want to go that way. (It is not a problem for PyPy but PyPy is not a simple JIT.) Well, I guess CPython is not ever going down that path anyway.

@arigo: But CPython is already committed to supporting writes to locals() at any moment, because at any moment you can set a trace function and in every proposal trace functions can reliably write to locals. So I don't think this is a serious obstacle for addng a JIT to CPython -- or at least, it doesn't add any new obstacles.

We're OK with the idea that installing a trace function might automatically turn off various compiler and interpreter managed optimisations (it's similar to how requesting or implying reliance on full frame support in other implementations can disable various optimisations). For trace hooks like coverage tools and debuggers, that's often downright desirable, since it makes the runtime behaviour correlate more directly with the source code.

What we're aiming to avoid is requiring that implementations make the assertion in "a = 1; locals('a') = 2; assert a == 2" pass at function scope, and if anything, we'd prefer to make it a requirement for that assertion to *fail*.

Requiring locals to actually *be* a write-through proxy (for all locals, not just cell references) would revert Python's semantics to the state they were in before the "fast locals" concept was first introduced, and we have no intention of going back there.

Thanks Nick for the clarification. Yes, that's what I meant: supporting such code in simple JITs is a nightmare. Perhaps more importantly, I am sure that if Python starts supporting random mutation of locals outside tracing hooks, then it would open the door to various hacks that are best not done at all, from a code quality point of view.

I'll also note there's a simpler reason the namespace object exposed at the function level can't just be a write-through proxy for the underlying frame: references to frame.f_locals may outlive the frame backing it, at which point we really do want it to be a plain dictionary with no special behaviour, just as it is for regular execution frames.

(Think "return locals()" as the last line in a helper function, as well as variants like "data = locals(); data.pop('some_key'); return data")

That means that no matter what, we need to snapshot the frame locals the when frame.f_locals is requested. The question then becomes:

• when we do we update the contents of cell references? (this is what's buggy right now when a trace function is installed)
• when do we update ordinary local variables? (this isn't broken, so we want to avoid changing it)

Providing write-through support *just* for cells should thus make it possible to fix the buggy interaction between cells and trace function, while minimising the risk of any unintended consequences affecting regular function locals.

Hm.... I may just be completely off here, but I thought that compilers could be allowed to recognize the use of locals() in a particular function and then disable JIT optimizations for that function. (In fact I thought we already had a rule like this but I can't find any language about it, but maybe I'm mistaken and we only have such an exception for sys._getframe() -- though it's not mentioned in the docs for that either.)

I do like Nathaniel's idea of making locals() a write-through proxy (and let f_locals the same thing). If this keeps the frame alive, well too bad -- there are lots of other things that do this too, e.g. tracebacks.

Or what about a read-only proxy or a plain snapshot? The docs already say that it *shouldn't* be written and *may* not write through -- are we concerned that a lot of people depend on the actual runtime behavior rather than the documented behavior?

Guido: you must be tired and forgot that locals() is a regular function :-) The compiler cannot recognize it reliably. Moreover, if f_locals can be modified outside a tracing hook, then we have the same problem in a cross-function way, e.g. if function f1() calls function f2() which does sys._getframe(1).f_locals['foo'] = 42.

Yep, my current goal is to see if I can come up with a surgical fix that solves the established problem with the bad interaction between cells and trace functions without any unintended consequences for either CPython or other interpreters.

That means the only behaviours I actually *want* to change are those that are pretty clearly quirky at best, and outright bugs at worst:

• registering a trace function can result in closure state being reset inappropriately, even when none of the code involved accesses locals() or f_locals
• registering a trace function may lead to changes to locals() made outside the trace function nevertheless being written back to the actual frame state

Establishing a write-through proxy for cell references is definitely fine - allowing shared access to closure state is the whole reason we have cell objects in the first place.

The more complex case is with regular locals since:

• they used to be regular dictionaries in 1.x, but one of the early 2.x releases deliberately changed their semantics with the introduction of fast locals
• people *do* sometimes treat the result of locals() at function scope as a regular dictionary, and hence they don't always copy it before mutating it and/or returning a reference to it
• f_locals is accessible from outside the running function/generator/coroutine, so compilers can't just key off calls to locals() inside the function to decide whether or not they can see all changes to local variables
• looking for calls to locals() at compile time is dubious anyway, since the builtin may have been aliased under a different name (we do something like that for zero-arg super(), but that breaks far more obviously when the use of name aliasing prevents the compiler from detecting that you need a __class__ reference compiled in)
• trace functions nevertheless still need to be able to write their changes back to the function locals in order for debuggers to support value injection

My latest design concept for the trace proxy thus looks like this (I've been iterating on design ideas to try to reduce the potential memory impact arising from merely installing a trace function):

1. The core proxy behaviour would be akin to wrapping f_locals in types.MappingProxyType (and I expect the new proxy will be a subclass of that existing type, with the tentative name "_FunctionLocalsProxy")

2. The currently planned differences consist of the following:

• setting and deleting items is supported
• holding a reference back to the originating frame (to allow for lazy initialisation of the extra state only if the local variables are actually mutated through the proxy)
• when a cell variable is mutated through the proxy, the cell gets added to a lazily constructed mapping from names to cells (if it isn't already there), and the value in the cell is also modified
• when a local variable is mutated through the proxy, it gets added to a set of "pending writebacks"

The post-traceback frame update in the trampoline function would then write back only the locals registered in "pending writebacks" (i.e. only those changes made through the proxy, *not* any incidental changes made directly to the result of locals()), which would allow this change to reduce the potential local state manipulation side effects of registering a trace function.

If actual implementation shows that this approach faces some technical hurdle that makes it infeasible in practice, then I agree it would make sense for us to look at alternatives with higher risks of unwanted side effects. However, based on what I learned while writing the first draft of PEP-558, I'm currently fairly optimistic I'll be able to make the idea work.

@nick:

We're OK with the idea that installing a trace function might automatically turn off various compiler and interpreter managed optimisations

We're OK with assigning to locals() doing that too.

What we're aiming to avoid is requiring that implementations make the assertion in "a = 1; locals('a') = 2; assert a == 2" pass at function scope, and if anything, we'd prefer to make it a requirement for that assertion to *fail*.

Why do you want it to fail? Given that it's already guaranteed to pass in some scopes (definitely in CPython, I'm not sure if it's in the language spec), and that we need to support mutations of f_locals for debugging, then this seems like a quixotic requirement. But if we do want it to fail, then we should make it fail obviously, e.g. by having locals() return a read-only mapping that raises an exception on mutation.

The only rationale I can understand here is the desire to leave options open for alternative implementations... but in practice all those alternative implementations can either support this just fine (because they have to to support pdb), or else they're MicroPython and have already stopped supporting locals() entirely so it hardly matters how CPython tweaks the details of the semantics.

Requiring locals to actually *be* a write-through proxy (for all locals, not just cell references) would revert Python's semantics to the state they were in before the "fast locals" concept was first introduced, and we have no intention of going back there.

Why not? I don't think anyone has ever suggested that "fast locals" made Python's *semantics* simpler.

@armin:

Perhaps more importantly, I am sure that if Python starts supporting random mutation of locals outside tracing hooks, then it would open the door to various hacks that are best not done at all, from a code quality point of view.

I'm not saying that assigning to locals() is great style, but there are totally legitimate use cases (debugging), and there's nothing stopping those hacks right now. You *can* use tracing hooks, or there's this nonsense: https://gist.github.com/njsmith/2347382

Once Python commits to supporting something, then generally it exposes it in a pretty straightforward way; it's really unusual to put the feature in a non-obvious place and then make the obvious way subtly broken to punish people who try to use it.

@nick

• when do we update ordinary local variables? (this isn't broken, so we want to avoid changing it)

But as pointed out earlier in this thread, pdb's support for updating ordinary local variables *is* currently broken, and would be fixed by using a write-through proxy for f_locals: https://bugs.python.org/issue30744#msg297480

The current pdb misbehaviour when navigating the stack is indeed a valid argument in favour of allowing immediate write-through from trace functions for regular local variables as well - I don't recall reading that bug report when Xavier first linked to it, so thanks for bringing it up again.

Another useful point that bug report highlights is that one of the assumptions I'd been making (that typical trace hooks would only need to manipulate the frame they were called for) is incorrect: via frame.f_back, the trace hook also has access to all of the parent frames, so any delayed write-back based approach would need to traverse the whole call stack to be truly robust. At that point, granting trace functions access to a write-through proxy as f_locals for *every* frame starts looking like a far more attractive option.

This suggests to me that rather than only switching behaviours while the traceback function is running, we would instead need to make it so that we check for whether or not a trace function is installed for the process whenever f_locals is looked up. If it is, and the frame uses fast locals, then f_locals will be a _FunctionLocalsProxy instance. Otherwise, it will be a regular dictionary.

Making a dynamic decision on whether to return a proxy or not is needed to handle the pdb stack traversal use case properly: debuggers are often going to be confronted with frames that were created *before* their trace hook was installed. Right now, as per the issue Xavier linked, attempted writes back to those don't tend to work properly (due to when and where the write-back happens), but dynamically upgrading to write-through proxies in the presence of a trace hook should help resolve that.

While that's a more pervasive change than what I was considering, I think the cited pdb bug provides a strong enough rationale to justify the related risk of unintended consequences.

Interestingly, I think it also gives us a way to reduce the degree to which installation of a trace hook affects the semantics of locals(): when f_locals on a frame is a _FunctionLocalsProxy instance, the locals() builtin can still bypass it and return the underlying dict. The difference between this and my earlier proposal for two different snapshot namespaces is that there'd still only be one snapshot, it's that f_locals would either reference it directly (the default state), or else via a write-through proxy (the "tracing mode" state).

I think that approach will give us the best possible outcome available:

• for code using locals(), the behaviour in non-tracing mode will be completely unchanged, and the semantic differences between tracing and non-tracing mode while a function is running will be reduced, since the post-trace-hook writeback code will be removed, and locals() itself never return a reference to the write-through proxy (only the underlying namespace snapshot). (There will still be some semantic differences, since the snapshot will be implicitly refreshed in tracing mode, whereas you have to call locals() explicitly to refresh it in non-tracing mode)

• for code using frame.f_locals, it will either behave the same as locals() if no tracing hook is installed when you retrieve a reference from the frame (again, no change from the status quo), *or* it will immediately write through to the active locals if a tracing hook *is* installed (replacing the current post-trace-hook writeback code)

(Note: if folks want to instead argue for the compatibility break option, you're going to have to write your own PEP, as I have zero plans to implement that myself, and will oppose it as a gratuitously unnecessary compatibility break if it does get proposed. Toggling this behaviour change on "Tracing or not?" won't be hard, and it ensures any unforeseen negative outcomes will only impact situations that are currently broken anyway)

@armin:

Guido: you must be tired and forgot that locals() is a regular function :-) The compiler cannot recognize it reliably.

I know, but we occasionally do make exceptions for builtins. In particular super() is recognized by the compiler.

@everyone else:
I need to limit my screen time this weekend, hopefully you can either figure this out without my help -- there's no way I can keep up with this discussion. Sorry!

Moreover, if f_locals can be modified outside a tracing hook, then we have the same problem in a cross-function way, e.g. if function f1() calls function f2() which does sys._getframe(1).f_locals['foo'] = 42.

That's covered by the rule that says sys._getframe() has undefined results.

Guido: any proposed semantic changes for CPython will eventually make their way through PEP-558 (and potentially a competing PEP if someone else is sufficiently keen on the idea of making non-tracing mode behave the same way as tracing mode), so ignoring the detailed blow-by-blow on this ticket should be fine.

If anything, it will make it easier for you to pick up any relevant details of the eventual rationale that the PEP text has missed.

Err, I phrased that last sentence really badly.

I meant that if Guido *hasn't* been following this ticket closely, he's more likely to ask for clarification if PEP-558 proposes a semantic change based on the discussion here without adequately justifying it in the PEP itself.

Sounds like a plan, I will ignore the ticket then.

Just confirming that the locals() vs frame.f_locals distinction also exists in the C API: the former is PyEval_GetLocals() (which implicitly ensures the snapshot is up to date), while the equivalent of the latter is direct access to frame->f_locals (and it's left to code accessing the field to decide whether it needs an up to date snapshot or not).

That means I'll be able to change what we store in frame->f_locals, while updating PyEval_GetLocals to check for a _FastLocalsProxy instance and return a reference to the underlying snapshot instead.

I rewrote the PEP based on the latest iteration of the design concept: python/peps@9a8e590

(That initial commit has some minor errors in it that I've already fixed, so check the latest version in the repo if you spot anything else - if I'd thought it through properly, I would have submitted the update as a PR, so folks had a place to more easily make line-by-line comments)

I genuinely like this version, and I think it should be reasonably straightforward to implement (given that types.MappingProxyType already exists).

Nick, in msg304388 you wrote "[allow] immediate write-through from trace functions". The latest iteration of PEP-558 says instead:

"tracing mode: the way the interpreter behaves when a trace hook has been registered..."

and also says:

"As long as the process remains in tracing mode, then __setitem__ and __delitem__ operations on the proxy will affect not only the dynamic snapshot, but also the corresponding fast local or cell reference on the underlying frame."

So write-through is allowed by the current PEP outside a trace function. Does not this change in some cases the behavior of code that updates the mapping returned by locals() when tracing mode is active from its standard behavior when there is no tracing ?

No, as locals() never returns the write-through proxy in the latest version of the PEP - it only ever returns the dynamic snapshot namespace (retrieving it from the proxy if necessary).

To get access to the write-through proxy in Python level code, you have to access frame.f_locals directly (which trace functions will do by necessity, since that's the only way they have to get at the function locals).

When I rejected that approach previously, it hadn't occurred to me yet that the write-through proxy could write through to both the frame state *and* the regular dynamic snapshot returned by locals().

The current design came from asking myself "What if the proxied reads always came from the snapshot, just like they do now, but writes went to *both* places?".

So far I haven't found any fundamental problems with the approach, but I also haven't implemented it yet - I've only read through all the places in the code where I think I'm going to have to make changes in order to get it to work.

Starting to make some progress on an implementation, and it occurs to me that if this approach does work out, it should make Python level trace functions *much* faster.

Right now, the call to the Python function in call_trampoline is bracketed by PyFrame_FastToLocals() and PyFrame_LocalsToFast(), even if the trace function never accesses frame.f_locals.

By contrast, with the proposed design, PyFrame_LocalsToFast() never gets called anywhere (I've actually replaced the entire body with a PyErr_SetString call), and PyFrame_FastTo_Locals will only be called in the frame.f_locals descriptor implementation.

This is tagged 3.10. It still fails on python3.10 but passes on 3.11 when using @arigo's minimal reproducer

import sys

def outer():
    x = 0

    def traced_looper():
        # Force w_locals to be instantiated (only matters on PyPy; on CPython
        # you can comment this line out and everything stays the same)
        locals()
        # Force x to be closed over (could use 'nonlocal' on py3)
        if False:
            x

    yield traced_looper
    while True:
        x += 1
        yield "value of x is: %d" % (x,)

outer_generator = outer()
traced_looper = next(outer_generator)


def tracefunc(frame, event, arg):
    print(next(outer_generator))
    return tracefunc

sys.settrace(tracefunc)
traced_looper()
sys.settrace(None)

which prints this for python3.10:

$ python3.10 /tmp/test_issue74929.py 
value of x is: 1
value of x is: 1
value of x is: 1
value of x is: 1

and the correct values for python3.11

$ python3.11 /tmp/test_issue74929.py 
value of x is: 1
value of x is: 2
value of x is: 3
value of x is: 4

The "fix" in 3.11 more by accident than design. I suspect that you could get the bug back by using sys.get_frame(1).f_locals in the tracer function.

I don't know if or when we will fix this properly.
My hope is that PEP 669 will kill off uses of sys.settrace and the problem will mostly go away.

Wouldn’t this also be fixed by different semantics for locals() per either Nick’s or Mark’s PEP for that?