Skip to content

bpo-1635741: _ast uses PyModule_AddObjectRef() - #23146

Merged
vstinner merged 1 commit into
python:masterfrom
vstinner:mod_addobjref_ast
Nov 4, 2020
Merged

bpo-1635741: _ast uses PyModule_AddObjectRef()#23146
vstinner merged 1 commit into
python:masterfrom
vstinner:mod_addobjref_ast

Conversation

@vstinner

@vstinner vstinner commented Nov 4, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Replace PyModule_AddObject() with PyModule_AddObjectRef() in the _ast
module (Python-ast.c) to fix a reference leak on error.

https://bugs.python.org/issue1635741
@vstinner
bpo-1635741: _ast uses PyModule_AddObjectRef()
f001865 
Replace PyModule_AddObject() with PyModule_AddObjectRef() in the _ast
module (Python-ast.c) to fix a reference leak on error.
@pablogsal

pablogsal commented Nov 4, 2020

Copy link
Copy Markdown
Member

Are you sure this is a reference leak? PyModule_AddObject steals a reference so there is no extra incref that has happened that is not decreased later. What this would fix if I understand correctly is a possible crash due to the refcount being less than needed.
@vstinner
vstinner merged commit 18ce7f1 into python:master Nov 4, 2020
@vstinner

vstinner commented Nov 4, 2020

Copy link
Copy Markdown
Member Author

Are you sure this is a reference leak? PyModule_AddObject steals a reference so there is no extra incref that has happened that is not decreased later. What this would fix if I understand correctly is a possible crash due to the refcount being less than needed.

I was thinking about the error path, but you're right: my change doesn't fix any leak. I was confused by other code that I modified locally which have a bug (I didn't create PRs for them yet).

I removed the mention about a leak in the commit message and I merged my PR.

I prefer to use PyModule_AddObjectRef() because it looks really weird to be to decrement the refcount to immediately increment it. It would be dangerous if it would be a borrowed reference with a reference count of 1! (but it's not the case here)
@vstinner
vstinner deleted the mod_addobjref_ast branch November 4, 2020 15:39
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Nov 5, 2020
@shihai1991
Merge branch 'master' into bpo_41832
c267e35 
* master:
  bpo-42260: Add _PyInterpreterState_SetConfig() (pythonGH-23158)
  Disable peg generator tests when building with PGO (pythonGH-23141)
  bpo-1635741: _sqlite3 uses PyModule_AddObjectRef() (pythonGH-23148)
  bpo-1635741: Fix PyInit_pyexpat() error handling (pythonGH-22489)
  bpo-42260: Main init modify sys.flags in-place (pythonGH-23150)
  bpo-1635741: Fix ref leak in _PyWarnings_Init() error path (pythonGH-23151)
  bpo-1635741: _ast uses PyModule_AddObjectRef() (pythonGH-23146)
  bpo-1635741: _contextvars uses PyModule_AddType() (pythonGH-23147)
  bpo-42260: Reorganize PyConfig (pythonGH-23149)
  bpo-1635741: Add PyModule_AddObjectRef() function (pythonGH-23122)
  bpo-42236: os.device_encoding() respects UTF-8 Mode (pythonGH-23119)
  bpo-42251: Add gettrace and getprofile to threading (pythonGH-23125)
  Enable signing of nuget.org packages and update to supported timestamp server (pythonGH-23132)
  Fix incorrect links in ast docs (pythonGH-23017)
  Add _PyType_GetModuleByDef (pythonGH-22835)
  Post 3.10.0a2
  bpo-41796: Call _PyAST_Fini() earlier to fix a leak (pythonGH-23131)
  bpo-42249: Fix writing binary Plist files larger than 4 GiB. (pythonGH-23121)
  bpo-40077: Convert mmap.mmap static type to a heap type (pythonGH-23108)
  Python 3.10.0a2
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@vstinner @adorilson
bpo-1635741: _ast uses PyModule_AddObjectRef() (pythonGH-23146)
d804003 
Replace PyModule_AddObject() with PyModule_AddObjectRef() in the _ast
module (Python-ast.c).
@kylotan kylotan mannequin mentioned this pull request Sep 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news

4 participants

@vstinner @pablogsal @the-knights-who-say-ni @bedevere-bot