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

Problems with PyImport_GetImporter() #109521

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

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
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.
@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
…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
…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>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 23, 2023
…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>
csm10495 pushed a commit to csm10495/cpython that referenced this issue Sep 28, 2023
…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
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
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to 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
Projects
None yet
Development

No branches or pull requests

1 participant