Skip to content

Conversation

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Dec 6, 2025

⚠️ Dear reviewers, to avoid collapsing the GitHub API with a lot of comments, please open PRs against the base branch with any suggestions or fixes if you are sure are not controversial ⚠️


📚 Documentation preview 📚: https://cpython-previews--142351.org.readthedocs.build/

DinoV and others added 30 commits October 2, 2025 13:22
Highlight lazy imports in the new REPL
@markshannon
Copy link
Member

@DinoV
What is the performance impact of adding a watcher to every module's dict?

Also, I'm bit concerned that with sufficient ingenuity, other cases will be found that expose lazy import objects. Although, I haven't found any as yet.

Flow in _PyInterpreterFrame and fix issue when global not defined
@DinoV
Copy link
Contributor

DinoV commented Jan 28, 2026

Nice catch! I've modified this to use a dictionary watcher instead so that we can catch all of the assignments to globals.

Arguably this is overkill... we say that we don't absolutely prevent the leak of lazy objects, and specifically call out globals() as one of the places where that happens. And so it feels similar to say if you sneak one in via globals() you may not get correct behavior either. But sneaking in does seem a little more egregious to me than leaking out.

The nice thing about using the dictionary watcher is it makes other things more full-proof - we don't need to catch the assignments and call the helper to clear the dict keys version.

@DinoV
Copy link
Contributor

DinoV commented Jan 28, 2026

I have made the requested changes; please review again.

@DinoV
Copy link
Contributor

DinoV commented Jan 28, 2026

@DinoV What is the performance impact of adding a watcher to every module's dict?

Also, I'm bit concerned that with sufficient ingenuity, other cases will be found that expose lazy import objects. Although, I haven't found any as yet.

The JIT already is using GLOBALS_WATCHER_ID to watch globals so I assume it's not going to be significant. It also only will impact writes to globals which should be infrequent so it additionally seems unlikely to have any impact.

But the goal isn't to 100% make sure that no lazy objects leak as mentioned in the PEP. So I think if there was a performance impact from this then we could go back to saying if you leak a lazy object and stick it into globals directly that you may get incorrect behavior.

I'll look at getting a perf run against the current branch to make sure the impact is as expected.

@StanFromIreland
Copy link
Member

FYI there was a bad merge.

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.