Skip to content

gh-141510: Add frozendict_check_mutable()#144944

Closed
vstinner wants to merge 2 commits intopython:mainfrom
vstinner:frozendict_mutable
Closed

gh-141510: Add frozendict_check_mutable()#144944
vstinner wants to merge 2 commits intopython:mainfrom
vstinner:frozendict_mutable

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Feb 18, 2026

frozendict.fromkeys() now checks if it can mutate the newly created frozendict.

frozendict.fromkeys() now checks if it can mutate the newly created
frozendict.
@vstinner
Copy link
Member Author

@methane @corona10: I wrote this change to raise an exception if a frozendict subclass constructor keeps a reference to the newly created dictionary.

I don't know which error message should be used, so I wrote: RuntimeError("cannot mutate frozendict already exposed in Python"). Do you have a better error message to propose?

@markshannon
Copy link
Member

Frozen dicts are immutable, that's the whole point of them, so why are we allowing them to be mutated?

Mutation of tuples has been an endless source of bugs, please don't do the same with frozen dicts.

@markshannon
Copy link
Member

I don't know which error message should be used, so I wrote: RuntimeError("cannot mutate frozendict already exposed in Python"). Do you have a better error message to propose?

This should be a SystemError.

@corona10
Copy link
Member

Frozen dicts are immutable, that's the whole point of them, so why are we allowing them to be mutated?

I concur with Mark's opinion here.

static int
frozendict_check_mutable(PyObject *self)
{
if (Py_REFCNT(self) > 1) {
Copy link
Member

@corona10 corona10 Feb 18, 2026

Choose a reason for hiding this comment

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

IIRC, Checking Py_REFCNT at runtime is not thread safe at free-threaded build.

@vstinner
Copy link
Member Author

This change fix the following example:

fd = None

class SpecialDict(frozendict):
    def __new__(self):
        global fd
        fd = frozendict(x=1)
        return fd

print(SpecialDict.fromkeys("y"))

Currently, _PyDict_FromKeys() calls SpecialDict.__new__() which runs arbitrary Python code.

This change detects the special constructor and raises an error.

Mutation of tuples has been an endless source of bugs, please don't do the same with frozen dicts.

An alternative is to modify _PyDict_FromKeys() to not call the subclass constructor, but always call frozendict constructor: I wrote #144947 to implement this idea.

A more radical fix would be to disallow subclassing frozendict.

@vstinner vstinner marked this pull request as draft February 18, 2026 11:27
@vstinner
Copy link
Member Author

frozendict_check_mutable() was a bad idea, I close the PR.

I wrote a 3rd PR to fix fromkeys(): #144952 copies the frozendict.

@vstinner vstinner closed this Feb 18, 2026
@vstinner vstinner deleted the frozendict_mutable branch February 18, 2026 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments