From 24ac184b6be5e5b46edfb36f12ca68aea580011c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 14 Nov 2018 01:31:31 +0100 Subject: [PATCH 1/3] bpo-35239: _PySys_EndInit() copies module_search_path * The _PySys_EndInit() function now copies the config->module_search_path list, so config is longer modified when sys.path is updated. * config->warnoptions list and config->xoptions dict are also copied * test_embed: InitConfigTests now also tests main_config['module_search_path'] --- Lib/test/test_embed.py | 8 +------- Programs/_testembed.c | 3 +-- Python/sysmodule.c | 24 +++++++++++++++++++++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 03a11842fa6f8a5..289809775708f12 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -269,10 +269,6 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): 'module_search_paths', 'prefix', ) - # FIXME: untested main configuration variables - UNTESTED_MAIN_CONFIG = ( - 'module_search_path', - ) DEFAULT_CORE_CONFIG = { 'install_signal_handlers': 1, 'use_environment': 1, @@ -388,15 +384,13 @@ def check_config(self, testname, expected): executable = core_config['executable'] main_config = config['main_config'] - for key in self.UNTESTED_MAIN_CONFIG: - del main_config[key] - expected_main = { 'install_signal_handlers': core_config['install_signal_handlers'], 'argv': [], 'prefix': sys.prefix, 'executable': core_config['executable'], 'base_prefix': sys.base_prefix, + 'module_search_path': core_config['module_search_paths'], 'base_exec_prefix': sys.base_exec_prefix, 'warnoptions': core_config['warnoptions'], 'xoptions': {}, diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 12dc0f98be45d12..2526b760f863b9b 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -202,10 +202,9 @@ static int test_pre_initialization_sys_options(void) _Py_EMBED_PREINIT_CHECK("Initializing interpreter\n"); _testembed_Py_Initialize(); _Py_EMBED_PREINIT_CHECK("Check sys module contents\n"); - PyRun_SimpleString("import sys; " + PyRun_SimpleString("import sys, warnings; " "print('sys.warnoptions:', sys.warnoptions); " "print('sys._xoptions:', sys._xoptions); " - "warnings = sys.modules['warnings']; " "latest_filters = [f[0] for f in warnings.filters[:3]]; " "print('warnings.filters[:3]:', latest_filters)"); _Py_EMBED_PREINIT_CHECK("Finalizing interpreter\n"); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 99cab2b4d9e3699..1b2b84c36f66d4a 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2488,7 +2488,20 @@ _PySys_EndInit(PyObject *sysdict, PyInterpreterState *interp) assert(config->exec_prefix != NULL); assert(config->base_exec_prefix != NULL); - SET_SYS_FROM_STRING_BORROW("path", config->module_search_path); +#define COPY_LIST(KEY, ATTR) \ + do { \ + assert(PyList_Check(config->ATTR)); \ + PyObject *list = PyList_GetSlice(config->ATTR, \ + 0, PyList_GET_SIZE(config->ATTR)); \ + if (list == NULL) { \ + return -1; \ + } \ + SET_SYS_FROM_STRING_BORROW(KEY, list); \ + Py_DECREF(list); \ + } while (0) + + COPY_LIST("path", module_search_path); + SET_SYS_FROM_STRING_BORROW("executable", config->executable); SET_SYS_FROM_STRING_BORROW("prefix", config->prefix); SET_SYS_FROM_STRING_BORROW("base_prefix", config->base_prefix); @@ -2505,10 +2518,15 @@ _PySys_EndInit(PyObject *sysdict, PyInterpreterState *interp) SET_SYS_FROM_STRING_BORROW("argv", config->argv); } if (config->warnoptions != NULL) { - SET_SYS_FROM_STRING_BORROW("warnoptions", config->warnoptions); + COPY_LIST("warnoptions", warnoptions); } if (config->xoptions != NULL) { - SET_SYS_FROM_STRING_BORROW("_xoptions", config->xoptions); + PyObject *dict = PyDict_Copy(config->xoptions); + if (dict == NULL) { + return -1; + } + SET_SYS_FROM_STRING_BORROW("_xoptions", dict); + Py_DECREF(dict); } /* Set flags to their final values */ From ece44166f42d41d5c03620acbabf8f2ec7b18d0e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 14 Nov 2018 01:58:49 +0100 Subject: [PATCH 2/3] Fix _Py_InitializeMainInterpreter() Don't use config->warnoptions but sys.warnoptions to decide if the warnings module should be imported at startup. --- Programs/_testembed.c | 3 ++- Python/pylifecycle.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 2526b760f863b9b..12dc0f98be45d12 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -202,9 +202,10 @@ static int test_pre_initialization_sys_options(void) _Py_EMBED_PREINIT_CHECK("Initializing interpreter\n"); _testembed_Py_Initialize(); _Py_EMBED_PREINIT_CHECK("Check sys module contents\n"); - PyRun_SimpleString("import sys, warnings; " + PyRun_SimpleString("import sys; " "print('sys.warnoptions:', sys.warnoptions); " "print('sys._xoptions:', sys._xoptions); " + "warnings = sys.modules['warnings']; " "latest_filters = [f[0] for f in warnings.filters[:3]]; " "print('warnings.filters[:3]:', latest_filters)"); _Py_EMBED_PREINIT_CHECK("Finalizing interpreter\n"); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 4ccea2ece207fbe..58e16473100e3a6 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -836,8 +836,8 @@ _Py_InitializeMainInterpreter(PyInterpreterState *interp, } /* Initialize warnings. */ - if (interp->config.warnoptions != NULL && - PyList_Size(interp->config.warnoptions) > 0) + PyObject *warnoptions = PySys_GetObject("warnoptions"); + if (warnoptions != NULL && PyList_Size(warnoptions) > 0) { PyObject *warnings_module = PyImport_ImportModule("warnings"); if (warnings_module == NULL) { From ec00512c6ec43411d5fc99dddcb9257a8411acd4 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 15 Nov 2018 05:43:07 +0100 Subject: [PATCH 3/3] post-merge fixes * test main_config['module_search_path'] * unset COPY_LIST --- Lib/test/test_embed.py | 11 +++-------- Python/sysmodule.c | 2 ++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 75e31c211122224..3f383d7fd50c8f4 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -331,10 +331,6 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): }) # main config - UNTESTED_MAIN_CONFIG = ( - # FIXME: untested main configuration variables - 'module_search_path', - ) COPY_MAIN_CONFIG = ( # Copy core config to main config for expected values 'argv', @@ -346,7 +342,8 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): 'prefix', 'pycache_prefix', 'warnoptions', - # xoptions is created from core_config in check_main_config() + # xoptions is created from core_config in check_main_config(). + # 'module_search_paths' is copied to 'module_search_path'. ) # global config @@ -426,12 +423,10 @@ def check_main_config(self, config): main_config = config['main_config'] # main config - for key in self.UNTESTED_MAIN_CONFIG: - del main_config[key] - expected_main = {} for key in self.COPY_MAIN_CONFIG: expected_main[key] = core_config[key] + expected_main['module_search_path'] = core_config['module_search_paths'] expected_main['xoptions'] = self.main_xoptions(core_config['xoptions']) self.assertEqual(main_config, expected_main) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 1b2b84c36f66d4a..2284e88d4c11800 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2529,6 +2529,8 @@ _PySys_EndInit(PyObject *sysdict, PyInterpreterState *interp) Py_DECREF(dict); } +#undef COPY_LIST + /* Set flags to their final values */ SET_SYS_FROM_STRING_INT_RESULT("flags", make_flags()); /* prevent user from creating new instances */