Add replacements for PySys_GetObject()

[serhiy-storchaka]

PySys_GetObject() has the following issues:

• It returns a borrowed reference. The result should be used with care with the GIL, but in the GIL-less build it is always unsafe.
• It silences all exceptions (including MemoryError and UnicodeDecodeError). This can lead to unexpected behavior in memory starving program, to hiding bugs, and maybe to suppressing user interruption. AFAIK, it is the last of the C API functions that has not get a replacement.
• It only accept the name as the C string. Usually we add the Object suffix to variants of functions that accept Python string, but PySys_GetObjectObject() would look weird.

There were several private variants (_PySys_GetObject(), _PySys_GetAttr() and others in older Python versions).

By analyzing the uses of PySys_GetObject() and its private variants, I have found, that in half of cases the name is passed as the C string, and in other half it is passed as the Python string. In half of cases the absent sys attribute is not treated as error, and in other hals a RuntimeError is raised (e.g. "lost sys.stdout"). So I came to the conclusion that four new functions are needed:

PyObject *PySys_GetAttr(PyObject *);
PyObject *PySys_GetAttrString(const char *);
int PySys_GetOptionalAttr(PyObject *, PyObject **);
int PySys_GetOptionalAttrString(const char *, PyObject **);

PySys_GetAttr() and PySys_GetAttrString() raise RuntimeError and return NULL if the sys attribute was not found. PySys_GetOptionalAttr() and PySys_GetOptionalAttrString() behave like other C API with "Optional" in the name -- they return 1 and set the strong reference if the sys attribute exists, return 0 (without raising an exception) if it does not exist, and return -1 if other error (like MemoryError or UnicodeDecodeError) occurred.

The number of usages per name:

Function	Usages
PySys_GetAttr	13
PySys_GetAttrString	16
PySys_GetOptionalAttr	22
PySys_GetOptionalAttrString	11

Unfortunately, no variant prevails, all functions are equally useful, so we cannot shorten this list. I tried to find better solution since I created an issue 1.5 years ago, but this is what I only have.

Issue: python/cpython#108512
PR: python/cpython#111035

I am ready to listen to your suggestions and advice, and then I will open a poll.

[serhiy-storchaka]

Removed usages in tests in the table.

[vstinner]

PySys_GetAttr() and PySys_GetAttrString() raise RuntimeError and return NULL if the sys attribute was not found.

I don't think that RuntimeError is the most appropriate error, I would prefer to raise AttributeError for a new API.

In half of cases the absent sys attribute is not treated as error, and in other half a RuntimeError is raised (e.g. "lost sys.stdout").

This is unfortunate. Can we modify this exception to AttributeError? I don't expect applications to rely on this RuntimeError since "it should not occur" (it's uncommon to delete sys attributes).

[vstinner]

Note: I'm fine with raising RuntimeError if the sys module (dictionary) cannot be retrieved.

[serhiy-storchaka]

If make a new API raising AttributeError, we will need to add additional convenient API that raises RuntimeError, not leaving a use case for API that raises AttributeError. So why bother to add it at first place?

If you want to change existing RuntimeErrors to AttributeErrors, this is a separate issue. I think that RuntimeError is better exception for event which "should not occur".

[encukou]

I also don't think RuntimeError is a good default for user code. (edit: and I lean toward not providing a default, leaving this to users)

Can we have only the Optional variants (without the Optional name), and keep the ones that raise RuntimeError internal? (Or don't add them at all, and do PyErr_SetString(PyExc_RuntimeError, "lost sys.X"); where needed?)

[serhiy-storchaka]

We can, but then it will be convenient to add also private helpers that raise RuntimeError and use them in half of cases. Then, if we want to make them public tomorrow, we'll have a naming problem.

[vstinner]

I'm afraid that the design of these new functions is based only on CPython legacy.

It seems like in CPython, it's common to have to ignore a missing sys attribute. So PySys_GetOptionalAttr() is proposed as a convenient helper for that. CPython tries to handle corner cases and special cases. Also, existing APIs raise RuntimeError for a missing attribute, whereas the attribute is supposed to exist.

Outside CPython, I expect that PySys_GetObject() is mostly used and this function ignores errors. I would suggest to replace it with a PySys_GetAttr() function which raises an AttributeError exception if the attribute doesn't exist.

We should treat CPython differently. CPython can easily use internal functions with a different behavior.

For example, in CPython, we usually ignore the fact that sys.stdout attribute doesn't exist. Outside CPython, I would expect a hard error (AttributeError) in this case, it's an unexpected case. Only sys.stdout = None should be expected (in which case no exception is raised, but None is returned).

---

A code search on PyPI top 8,000 projects for \bPySys_GetObject\b regex found 28 projects:

• Nuitka (2.6)
• PyQt5 (5.15.11)
• PyQt6 (6.8.0)
• apsw (3.48.0.0)
• catboost (1.2.7)
• cffi (1.17.1)
• cx_freeze (7.2.8)
• cython (3.0.11)
• gevent (24.11.1)
• greenlet (3.1.1)
• guppy3 (3.1.5)
• memray (1.15.0)
• msgspec (0.19.0)
• nanobind (2.4.0)
• numpy (2.2.2)
• orjson (3.10.15)
• pemja (0.4.2)
• pillow (11.1.0)
• pygments (2.19.1)
• pyinstaller (6.11.1)
• pyqt5_sip (12.16.1)
• pythonnet (3.0.5)
• pyuwsgi-2.0.28.post1
• shap (0.46.0)
• sip (6.9.1)
• uwsgi (2.0.28)
• wxPython (4.2.2)
• zodbpickle (4.1.1)

Argument to PySys_GetObject():

• "apsw_fault_inject_control" -- doesn't exist in CPython
• "apsw_should_fault" -- doesn't exist in CPython
• "argv"
• "clr_data" -- doesn't exist in CPython
• "displayhook"
• "__excepthook__"
• "excepthook"
• "executable"
• "flags"
• "_getframe"
• "getobjects"
• "gettotalrefcount"
• "hexversion"
• "initial_modules" -- doesn't exist in CPython
• "meta_path"
• "modules"
• "path"
• "ps1"
• "stderr"
• "stdout"
• "unraisablehook"
• "version"
• "version_info"

---

A code search on PyPI top 8,000 projects for _PySys_GetObject|_PySys_GetAttr regex found 1 line in 1 project:

pickle5-0.0.12.tar.gz: pickle5-0.0.12/pickle5/_pickle.c: modules = _PySys_GetObjectId(&PyId_modules);

This code seems to be outdated since _PySys_GetObjectId() has been removed in Python 3.11.

So these private functions _PySys are not used outside Python.

[encukou]

it's common to have to ignore a missing sys attribute. So PySys_GetOptionalAttr() is proposed as a convenient helper for that.

I don't see it that way. IMO, PySys_GetOptionalAttr() leaves the error-handling to the user. Commonly, an error other than AttributeError is appropriate; with the Optional function you can skip both allocation of an exception object and a PyErr_ExceptionMatches call.

[malemburg]

There's a common use case for this:

Object finalizer will sometimes find that the sys module (and/or other modules) already have been fully or partially finalized, with their dictionaries cleared. In those cases, you typically want to continue processing as far as you can go under those circumstances to free up resources.

[serhiy-storchaka]

Can we have only the Optional variants (without the Optional name), and keep the ones that raise RuntimeError internal?

python/cpython#129736 implements this. Internal helpers _PySys_GetRequiredAttr() and _PySys_GetRequiredAttrString() are used in many places for convenience.

There are more issues with PySys_GetObject() in the GIL-less build (see python/cpython#130163). PySys_GetObject() should be replaced in maintained versions too.

It's better to decide about the public replacements for PySys_GetObject() sooner so we can backport them (as private functions) and avoid unnecessary code churn.

[sergey-miryanov]

To be accurate in python/cpython#130282 I reproduced the segfaults for the GIL-full build.

[encukou]

Thank you for making the PR, and ... sorry for making you do the work: I changed my mind.
(I intended it as a thought excercise argument, not an assignment.)

I still think the RuntimeError/AttributeError/ignore choice is best left to callers. (For backwards compatibility, RuntimeError would be CPython's backwards compatible default, but it's not that appropriate for public API.)

Then again, sys doesn't have that many optional attributes. The “not found” case would often only happen if someone deletes something from sys -- not really a case you'd test for.

So, if we do only add one set of functions, they should probably still be called PySys_GetOptionalAttr/PySys_GetOptionalAttrString, to make it clear that they can return 0 without an exception set.

Then, not having PySys_GetAttr would look like a weird omission.
Maybe we could have three sets of functions?

• PySys_GetAttr/PySys_GetAttrString - raise AttributeError if not found
• PySys_GetOptionalAttr/PySys_GetOptionalAttrString - return -1/0/1
• _PySys_GetAttr_RuntimeError/_PySys_GetAttrString_RuntimeError - raise RuntimeError if not found

That looks cumbersome.
Going back to the “not found” case often only possible if Python is “broken” by someone deleting something they shouldn't, perhaps the exception type doesn't matter that much. RuntimeError is fine.

So, I'm now +1 for the original proposal. RuntimeError is not ideal, but in cases where it matters, one can use the Optional variant.