Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

it's no longer possible to delete items from f_locals (FrameLocalsProxy) in 3.13+ #125590

Open
deanagarrott opened this issue Oct 16, 2024 · 10 comments
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@deanagarrott
Copy link

deanagarrott commented Oct 16, 2024

Bug report

Bug description:

           try:
                driver.switch_to.alert.accept()
                driver.switch_to.alert.dismiss()
            except:
                pass

In the example script above, if the code falls into the except: pass block due to an alert not being displayed, the script will ultimately fail (even if no coding errors are encountered) with FAILED (errors=1) .
Also, the following messages will be displayed in the output:
image

This code works fine in Python 3.12. The script succeeds with message OK and none of the above error messages. We can't move to 3.13 version until this is fixed.

CPython versions tested on:

3.13

Operating systems tested on:

Windows

Linked PRs

@deanagarrott deanagarrott added the type-bug An unexpected behavior, bug, or error label Oct 16, 2024
@JelleZijlstra
Copy link
Member

This sounds like an issue in the version of pydevd bundled by the Microsoft Python extension.

I assume it's related to the FrameLocalsProxy changes from PEP-667; cc @gaogaotiantian.

@ZeroIntensity
Copy link
Member

Maybe FrameLocalsProxy should just implement pop? (Or better yet, the rest of the dict interface.)

@ZeroIntensity ZeroIntensity added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Oct 16, 2024
@graingert
Copy link
Contributor

graingert commented Oct 16, 2024

Maybe FrameLocalsProxy should just implement pop? (Or better yet, the rest of the dict interface.)

problem is you cannot remove items from FrameLocalsProxy:

PyErr_SetString(PyExc_TypeError, "cannot remove variables from FrameLocalsProxy");

I think pydevd has to do

try:
    del frame.f_locals["__exception__"]
except KeyError:
    pass
except TypeError:
    frame.f_locals["__exception__"] = None

@gaogaotiantian
Copy link
Member

The reason why FrameLocalsProxy did not support pop and del in the first place, is because it does not make sense to "remove" a local variable from the frame. However, in this specific case, the debugger is trying to remove an extra variable, which is artificially added. I think it might be fine to check if it's an actual local variable and only report error when it is.

@markshannon and @gvanrossum as this was the discussion result from PEP 667 implementation.

@ZeroIntensity
Copy link
Member

I'm not particularly familiar with how PEP 667 was implemented. From my understanding, deleting from f_locals was previously possible--is it that it can't be implemented right now, or is it that it's dangerous to expose to users?

@markshannon
Copy link
Member

Popping and deleting keys in the "extras" dict should be fine.
It gets complicated when the key is a "fast" local.

In the style of the PEP, this would give us:

    def __delitem__(self, name):
        del self._extra_locals[name]

    def pop(self, name, default):
        return self._extra_locals.pop(name, default)

@gaogaotiantian
Copy link
Member

The semantics of f_locals has changed. f_locals used to be a pure dict which was written back to the real locals on certain occasions (the most common one is returning from trace functions). The old behavior, when you remove a key from f_locals, if it was written back, was to set the local variable to None and generate a warning - a very inconsistent and confusing behavior.

@gaogaotiantian
Copy link
Member

Popping and deleting keys in the "extras" dict should be fine. It gets complicated when the key is a "fast" local.

Let's allow popping and deleting keys in extras, and generate an exception (we are generating one now on del) if the user tries to remove a real local variable. Do we consider this a bug so we need to backport it to 3.13?

@graingert graingert changed the title try except pass results in a script failure and system errors when code falls into pass logic - new bug in Python 3.13 it's no longer possible to delete items from f_locals (FrameLocalProxy) in 3.13+ Oct 16, 2024
@graingert graingert changed the title it's no longer possible to delete items from f_locals (FrameLocalProxy) in 3.13+ it's no longer possible to delete items from f_locals (FrameLocalsProxy) in 3.13+ Oct 16, 2024
@graingert
Copy link
Contributor

The semantics of f_locals has changed. f_locals used to be a pure dict which was written back to the real locals on certain occasions (the most common one is returning from trace functions). The old behavior, when you remove a key from f_locals, if it was written back, was to set the local variable to None and generate a warning - a very inconsistent and confusing behavior.

how do you generate the warning?

I tried this on 3.12 and didn't see a warning, and the local didn't get set to None:

import sys

class A:
    pass


def foo():
    a = A()
    f_locals = sys._getframe().f_locals
    del f_locals['a']
    print(a)
    print(f_locals)


foo()
 graingert@conscientious  ~/projects  python3.12 -Werror demo.py
<__main__.A object at 0x7ae782f40230>
{}
 graingert@conscientious  ~/projects  python3.13 -Werror demo.py
Traceback (most recent call last):
  File "/home/graingert/projects/demo.py", line 15, in <module>
    foo()
    ~~~^^
  File "/home/graingert/projects/demo.py", line 10, in foo
    del f_locals['a']
        ~~~~~~~~^^^^^
TypeError: cannot remove variables from FrameLocalsProxy

@gaogaotiantian
Copy link
Member

I tried this on 3.12 and didn't see a warning, and the local didn't get set to None:

This is the case where the f_locals is not written back. Like I said, it was only written back on certain occasions, for example, when it returns from a trace function. And that's why we need to change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants