Skip to content

Problems with PyImport_GetImporter() #109521

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.

Sign up for GitHub

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

Jump to bottom
Closed
serhiy-storchaka opened this issue Sep 17, 2023 · 1 comment
Closed

Problems with PyImport_GetImporter() #109521

serhiy-storchaka opened this issue Sep 17, 2023 · 1 comment
Labels
topic-C-API type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Sep 17, 2023 •
edited by bedevere-app bot
Loading

Bug report

  1. PyImport_GetImporter() can return NULL with set or not set error. The latter happens only if sys.path_hooks or sys.path_importer_cache was deleted, or in more obscure cases: when string allocation for strings "path_hooks" or "path_importer_cache" fail, or the sys module was not yet created. These cases so obscure, that the user code most likely do not expect this. The only place where PyImport_GetImporter() is used in Python itself expects an error to be set if NULL is returned.
  2. PyImport_GetImporter() can crash (in debug build) or raise SystemError if sys.path_hooks is not a list or sys.path_importer_cache is not a dict. Note that both are set to None in _PyImport_FiniExternal() (which is called in Py_FinalizeEx() and Py_EndInterpreter()).

Crash is unacceptable, so asserts should be replaced with runtime checks and raising an exception (RuntimeError looks suitable).

And I think that PyImport_GetImporter() should set RuntimeError when it fails to get sys.path_hooks or sys.path_importer_cache. It is the most common error in similar cases.

Linked PRs
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump topic-C-API labels Sep 17, 2023
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 17, 2023
@serhiy-storchaka
pythongh-109521: Fix obscure cases handling in PyImport_GetImporter()
4542dec 
PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingle.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
@bedevere-app bedevere-app bot mentioned this issue Sep 17, 2023
Merged
@serhiy-storchaka
Copy link
Member Author

@ncoghlan, as the author of PyImport_GetImporter(), what can you say about this? Was there a case for PyImport_GetImporter() returning NULL without setting error, or was it just an oversight?

serhiy-storchaka added a commit that referenced this issue Sep 23, 2023
@serhiy-storchaka
gh-109521: Fix obscure cases handling in PyImport_GetImporter() (GH-1…
62c7015 
…09522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 23, 2023
@serhiy-storchaka @miss-islington
pythongh-109521: Fix obscure cases handling in PyImport_GetImporter() (…
04c479c 
…pythonGH-109522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
(cherry picked from commit 62c7015)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue Sep 23, 2023
Merged
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 23, 2023
@serhiy-storchaka
[3.11] pythongh-109521: Fix obscure cases handling in PyImport_GetImp…
10337d0 
…orter() (pythonGH-109522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type..
(cherry picked from commit 62c7015)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue Sep 23, 2023
Merged
csm10495 pushed a commit to csm10495/cpython that referenced this issue Sep 28, 2023
@serhiy-storchaka @csm10495
pythongh-109521: Fix obscure cases handling in PyImport_GetImporter() (…
309ff27 
…pythonGH-109522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
Yhg1s pushed a commit that referenced this issue Oct 2, 2023
@miss-islington @serhiy-storchaka
[3.12] gh-109521: Fix obscure cases handling in PyImport_GetImporter() (
5e6e996 
GH-109522) (#109777)

gh-109521: Fix obscure cases handling in PyImport_GetImporter() (GH-109522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
(cherry picked from commit 62c7015)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this issue Oct 7, 2023
@serhiy-storchaka
[3.11] gh-109521: Fix obscure cases handling in PyImport_GetImporter() (
6a33529 
GH-109522) (GH-109781)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
(cherry picked from commit 62c7015)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@serhiy-storchaka @Glyphack
pythongh-109521: Fix obscure cases handling in PyImport_GetImporter() (…
b8e03d9 
…pythonGH-109522)

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks
or sys.path_importer_cache or they are not list and dict correspondingly.

Previously it could return NULL without setting error in obscure cases,
crash or raise SystemError if these attributes have wrong type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-C-API type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@serhiy-storchaka