bpo-32030: Enhance Py_Main() (#4412)

Parse more env vars in Py_Main():

* Add more options to _PyCoreConfig:

  * faulthandler
  * tracemalloc
  * importtime

* Move code to parse environment variables from _Py_InitializeCore()
  to Py_Main(). This change fixes a regression from Python 3.6:
  PYTHONUNBUFFERED is now read before calling pymain_init_stdio().
* _PyFaulthandler_Init() and _PyTraceMalloc_Init() now take an
  argument to decide if the module has to be enabled at startup.
* tracemalloc_start() is now responsible to check the maximum number
  of frames.

Other changes:

* Cleanup Py_Main():

  * Rename some pymain_xxx() subfunctions
  * Add pymain_run_python() subfunction

* Cleanup Py_NewInterpreter()
* _PyInterpreterState_Enable() now reports failure
* init_hash_secret() now considers pyurandom() failure as an "user
  error": don't fail with abort().
* pymain_optlist_append() and pymain_strdup() now sets err on memory
  allocation failure.

Author: vstinner

Include/internal/pystate.h

@@ -90,7 +90,7 @@ PyAPI_FUNC(_PyInitError) _PyRuntime_Initialize(void);
 
 /* Other */
 
-PyAPI_FUNC(void) _PyInterpreterState_Enable(_PyRuntimeState *);
+PyAPI_FUNC(_PyInitError) _PyInterpreterState_Enable(_PyRuntimeState *);
 
 #ifdef __cplusplus
 }

Include/pydebug.h

@@ -25,6 +25,7 @@ PyAPI_DATA(int) Py_HashRandomizationFlag;
 PyAPI_DATA(int) Py_IsolatedFlag;
 
 #ifdef MS_WINDOWS
+PyAPI_DATA(int) Py_LegacyWindowsFSEncodingFlag;
 PyAPI_DATA(int) Py_LegacyWindowsStdioFlag;
 #endif
 

Include/pystate.h

@@ -30,9 +30,20 @@ typedef struct {
     unsigned long hash_seed;
     int _disable_importlib; /* Needed by freeze_importlib */
     char *allocator;
+    int faulthandler;
+    int tracemalloc;        /* Number of saved frames, 0=don't trace */
+    int importtime;         /* -X importtime */
 } _PyCoreConfig;
 
-#define _PyCoreConfig_INIT {0, -1, 0, 0, NULL}
+#define _PyCoreConfig_INIT \
+    {.ignore_environment = 0, \
+     .use_hash_seed = -1, \
+     .hash_seed = 0, \
+     ._disable_importlib = 0, \
+     .allocator = NULL, \
+     .faulthandler = 0, \
+     .tracemalloc = 0, \
+     .importtime = 0}
 
 /* Placeholders while working on the new configuration API
  *

Lib/test/test_tracemalloc.py

@@ -829,16 +829,23 @@ def test_env_limit(self):
         stdout = stdout.rstrip()
         self.assertEqual(stdout, b'10')
 
+    def check_env_var_invalid(self, nframe):
+        with support.SuppressCrashReport():
+            ok, stdout, stderr = assert_python_failure(
+                '-c', 'pass',
+                PYTHONTRACEMALLOC=str(nframe))
+
+        if b'ValueError: the number of frames must be in range' in stderr:
+            return
+        if b'PYTHONTRACEMALLOC: invalid number of frames' in stderr:
+            return
+        self.fail(f"unexpeced output: {stderr!a}")
+
+
     def test_env_var_invalid(self):
         for nframe in (-1, 0, 2**30):
             with self.subTest(nframe=nframe):
-                with support.SuppressCrashReport():
-                    ok, stdout, stderr = assert_python_failure(
-                        '-c', 'pass',
-                        PYTHONTRACEMALLOC=str(nframe))
-                    self.assertIn(b'PYTHONTRACEMALLOC: invalid '
-                                  b'number of frames',
-                                  stderr)
+                self.check_env_var_invalid(nframe)
 
     def test_sys_xoptions(self):
         for xoptions, nframe in (
@@ -852,15 +859,21 @@ def test_sys_xoptions(self):
                 stdout = stdout.rstrip()
                 self.assertEqual(stdout, str(nframe).encode('ascii'))
 
+    def check_sys_xoptions_invalid(self, nframe):
+        args = ('-X', 'tracemalloc=%s' % nframe, '-c', 'pass')
+        with support.SuppressCrashReport():
+            ok, stdout, stderr = assert_python_failure(*args)
+
+        if b'ValueError: the number of frames must be in range' in stderr:
+            return
+        if b'-X tracemalloc=NFRAME: invalid number of frames' in stderr:
+            return
+        self.fail(f"unexpeced output: {stderr!a}")
+
     def test_sys_xoptions_invalid(self):
         for nframe in (-1, 0, 2**30):
             with self.subTest(nframe=nframe):
-                with support.SuppressCrashReport():
-                    args = ('-X', 'tracemalloc=%s' % nframe, '-c', 'pass')
-                    ok, stdout, stderr = assert_python_failure(*args)
-                    self.assertIn(b'-X tracemalloc=NFRAME: invalid '
-                                  b'number of frames',
-                                  stderr)
+                self.check_sys_xoptions_invalid(nframe)
 
     @unittest.skipIf(_testcapi is None, 'need _testcapi')
     def test_pymem_alloc0(self):

Modules/_tracemalloc.c

@@ -1066,8 +1066,16 @@ tracemalloc_start(int max_nframe)
     PyMemAllocatorEx alloc;
     size_t size;
 
-    if (tracemalloc_init() < 0)
+    if (max_nframe < 1 || max_nframe > MAX_NFRAME) {
+        PyErr_Format(PyExc_ValueError,
+                     "the number of frames must be in range [1; %i]",
+                     (int)MAX_NFRAME);
         return -1;
+    }
+
+    if (tracemalloc_init() < 0) {
+        return -1;
+    }
 
     if (tracemalloc_config.tracing) {
         /* hook already installed: do nothing */
@@ -1500,7 +1508,7 @@ _PyMem_DumpTraceback(int fd, const void *ptr)
 /*[clinic input]
 _tracemalloc.start
 
-    nframe: Py_ssize_t = 1
+    nframe: int = 1
     /
 
 Start tracing Python memory allocations.
@@ -1510,22 +1518,12 @@ trace to nframe.
 [clinic start generated code]*/
 
 static PyObject *
-_tracemalloc_start_impl(PyObject *module, Py_ssize_t nframe)
-/*[clinic end generated code: output=0f558d2079511553 input=997841629cc441cb]*/
+_tracemalloc_start_impl(PyObject *module, int nframe)
+/*[clinic end generated code: output=caae05c23c159d3c input=40d849b5b29d1933]*/
 {
-    int nframe_int;
-
-    if (nframe < 1 || nframe > MAX_NFRAME) {
-        PyErr_Format(PyExc_ValueError,
-                     "the number of frames must be in range [1; %i]",
-                     (int)MAX_NFRAME);
+    if (tracemalloc_start(nframe) < 0) {
         return NULL;
     }
-    nframe_int = Py_SAFE_DOWNCAST(nframe, Py_ssize_t, int);
-
-    if (tracemalloc_start(nframe_int) < 0)
-        return NULL;
-
     Py_RETURN_NONE;
 }
 
@@ -1658,87 +1656,13 @@ PyInit__tracemalloc(void)
 }
 
 
-static int
-parse_sys_xoptions(PyObject *value)
-{
-    PyObject *valuelong;
-    long nframe;
-
-    if (value == Py_True)
-        return 1;
-
-    assert(PyUnicode_Check(value));
-    if (PyUnicode_GetLength(value) == 0)
-        return -1;
-
-    valuelong = PyLong_FromUnicodeObject(value, 10);
-    if (valuelong == NULL)
-        return -1;
-
-    nframe = PyLong_AsLong(valuelong);
-    Py_DECREF(valuelong);
-    if (nframe == -1 && PyErr_Occurred())
-        return -1;
-
-    if (nframe < 1 || nframe > MAX_NFRAME)
-        return -1;
-
-    return Py_SAFE_DOWNCAST(nframe, long, int);
-}
-
-
 int
-_PyTraceMalloc_Init(void)
+_PyTraceMalloc_Init(int nframe)
 {
-    char *p;
-    int nframe;
-
     assert(PyGILState_Check());
-
-    if ((p = Py_GETENV("PYTHONTRACEMALLOC")) && *p != '\0') {
-        char *endptr = p;
-        long value;
-
-        errno = 0;
-        value = strtol(p, &endptr, 10);
-        if (*endptr != '\0'
-            || value < 1
-            || value > MAX_NFRAME
-            || errno == ERANGE)
-        {
-            Py_FatalError("PYTHONTRACEMALLOC: invalid number of frames");
-            return -1;
-        }
-
-        nframe = (int)value;
-    }
-    else {
-        PyObject *xoptions, *key, *value;
-
-        xoptions = PySys_GetXOptions();
-        if (xoptions == NULL)
-            return -1;
-
-        key = PyUnicode_FromString("tracemalloc");
-        if (key == NULL)
-            return -1;
-
-        value = PyDict_GetItemWithError(xoptions, key); /* borrowed */
-        Py_DECREF(key);
-        if (value == NULL) {
-            if (PyErr_Occurred())
-                return -1;
-
-            /* -X tracemalloc is not used */
-            return 0;
-        }
-
-        nframe = parse_sys_xoptions(value);
-        if (nframe < 0) {
-            Py_FatalError("-X tracemalloc=NFRAME: invalid number of frames");
-        }
+    if (nframe == 0) {
+        return 0;
     }
-
     return tracemalloc_start(nframe);
 }
 

Modules/clinic/_tracemalloc.c.h

@@ -87,15 +87,15 @@ PyDoc_STRVAR(_tracemalloc_start__doc__,
     {"start", (PyCFunction)_tracemalloc_start, METH_FASTCALL, _tracemalloc_start__doc__},
 
 static PyObject *
-_tracemalloc_start_impl(PyObject *module, Py_ssize_t nframe);
+_tracemalloc_start_impl(PyObject *module, int nframe);
 
 static PyObject *
 _tracemalloc_start(PyObject *module, PyObject **args, Py_ssize_t nargs)
 {
     PyObject *return_value = NULL;
-    Py_ssize_t nframe = 1;
+    int nframe = 1;
 
-    if (!_PyArg_ParseStack(args, nargs, "|n:start",
+    if (!_PyArg_ParseStack(args, nargs, "|i:start",
         &nframe)) {
         goto exit;
     }
@@ -185,4 +185,4 @@ _tracemalloc_get_traced_memory(PyObject *module, PyObject *Py_UNUSED(ignored))
 {
     return _tracemalloc_get_traced_memory_impl(module);
 }
-/*[clinic end generated code: output=c9a0111391b3ec45 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=db4f909464c186e2 input=a9049054013a1b77]*/

Modules/faulthandler.c

@@ -1299,36 +1299,8 @@ faulthandler_init_enable(void)
     return 0;
 }
 
-/* Call faulthandler.enable() if the PYTHONFAULTHANDLER environment variable
-   is defined, or if sys._xoptions has a 'faulthandler' key. */
-
-static int
-faulthandler_init_parse(void)
-{
-    char *p = Py_GETENV("PYTHONFAULTHANDLER");
-    if (p && *p != '\0') {
-        return 1;
-    }
-
-    /* PYTHONFAULTHANDLER environment variable is missing
-       or an empty string */
-    PyObject *xoptions = PySys_GetXOptions();
-    if (xoptions == NULL) {
-        return -1;
-    }
-
-    PyObject *key = PyUnicode_FromString("faulthandler");
-    if (key == NULL) {
-        return -1;
-    }
-
-    int has_key = PyDict_Contains(xoptions, key);
-    Py_DECREF(key);
-    return has_key;
-}
-
 _PyInitError
-_PyFaulthandler_Init(void)
+_PyFaulthandler_Init(int enable)
 {
 #ifdef HAVE_SIGALTSTACK
     int err;
@@ -1357,11 +1329,6 @@ _PyFaulthandler_Init(void)
     PyThread_acquire_lock(thread.cancel_event, 1);
 #endif
 
-    int enable = faulthandler_init_parse();
-    if (enable < 0) {
-        return _Py_INIT_ERR("failed to parse faulthandler env var and cmdline");
-    }
-
     if (enable) {
         if (faulthandler_init_enable() < 0) {
             return _Py_INIT_ERR("failed to enable faulthandler");