Skip to content

Commit

Permalink
pythongh-96853: Restore test coverage for Py_Initialize(Ex) (pythonGH…
Browse files Browse the repository at this point in the history
…-98212)

* As most of `test_embed` now uses `Py_InitializeFromConfig`, add
  a specific test case to cover `Py_Initialize` (and `Py_InitializeEx`)
* Rename `_testembed` init helper to clarify the API used
* Add a `PyConfig_Clear` call in `Py_InitializeEx` to make
  the code more obviously correct (it already didn't leak as
  none of the dynamically allocated config fields were being
  populated, but it's clearer if the wrappers follow the
  documented API usage guidelines)
(cherry picked from commit 05e4886)

Co-authored-by: Nick Coghlan <[email protected]>
  • Loading branch information
ncoghlan authored and miss-islington committed Oct 30, 2022
1 parent a55bd6f commit c9b71f7
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 19 deletions.
6 changes: 6 additions & 0 deletions Lib/test/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,12 @@ def test_finalize_structseq(self):
out, err = self.run_embedded_interpreter("test_repeated_init_exec", code)
self.assertEqual(out, 'Tests passed\n' * INIT_LOOPS)

def test_simple_initialization_api(self):
# _testembed now uses Py_InitializeFromConfig by default
# This case specifically checks Py_Initialize(Ex) still works
out, err = self.run_embedded_interpreter("test_repeated_simple_init")
self.assertEqual(out, 'Finalized\n' * INIT_LOOPS)

def test_quickened_static_code_gets_unquickened_at_Py_FINALIZE(self):
# https://github.com/python/cpython/issues/92031

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
``Py_InitializeEx`` now correctly calls ``PyConfig_Clear`` after initializing
the interpreter (the omission didn't cause a memory leak only because none
of the dynamically allocated config fields are populated by the wrapper
function)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added explicit coverage of ``Py_Initialize`` (and hence ``Py_InitializeEx``)
back to the embedding tests (all other embedding tests migrated to
``Py_InitializeFromConfig`` in Python 3.11)
62 changes: 43 additions & 19 deletions Programs/_testembed.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ char **main_argv;
/*********************************************************
* Embedded interpreter tests that need a custom exe
*
* Executed via 'EmbeddingTests' in Lib/test/test_capi.py
* Executed via Lib/test/test_embed.py
*********************************************************/

// Use to display the usage
Expand Down Expand Up @@ -73,14 +73,20 @@ static void init_from_config_clear(PyConfig *config)
}


static void _testembed_Py_Initialize(void)
static void _testembed_Py_InitializeFromConfig(void)
{
PyConfig config;
_PyConfig_InitCompatConfig(&config);
config_set_program_name(&config);
init_from_config_clear(&config);
}

static void _testembed_Py_Initialize(void)
{
Py_SetProgramName(PROGRAM_NAME);
Py_Initialize();
}


/*****************************************************
* Test repeated initialisation and subinterpreters
Expand Down Expand Up @@ -110,7 +116,7 @@ static int test_repeated_init_and_subinterpreters(void)

for (int i=1; i <= INIT_LOOPS; i++) {
printf("--- Pass %d ---\n", i);
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
mainstate = PyThreadState_Get();

PyEval_ReleaseThread(mainstate);
Expand Down Expand Up @@ -168,7 +174,7 @@ static int test_repeated_init_exec(void)
fprintf(stderr, "--- Loop #%d ---\n", i);
fflush(stderr);

_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
int err = PyRun_SimpleString(code);
Py_Finalize();
if (err) {
Expand All @@ -178,6 +184,23 @@ static int test_repeated_init_exec(void)
return 0;
}

/****************************************************************************
* Test the Py_Initialize(Ex) convenience/compatibility wrappers
***************************************************************************/
// This is here to help ensure there are no wrapper resource leaks (gh-96853)
static int test_repeated_simple_init(void)
{
for (int i=1; i <= INIT_LOOPS; i++) {
fprintf(stderr, "--- Loop #%d ---\n", i);
fflush(stderr);

_testembed_Py_Initialize();
Py_Finalize();
printf("Finalized\n"); // Give test_embed some output to check
}
return 0;
}


/*****************************************************
* Test forcing a particular IO encoding
Expand All @@ -199,7 +222,7 @@ static void check_stdio_details(const char *encoding, const char * errors)
fflush(stdout);
/* Force the given IO encoding */
Py_SetStandardStreamEncoding(encoding, errors);
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
PyRun_SimpleString(
"import sys;"
"print('stdin: {0.encoding}:{0.errors}'.format(sys.stdin));"
Expand Down Expand Up @@ -308,7 +331,7 @@ static int test_pre_initialization_sys_options(void)
dynamic_xoption = NULL;

_Py_EMBED_PREINIT_CHECK("Initializing interpreter\n");
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
_Py_EMBED_PREINIT_CHECK("Check sys module contents\n");
PyRun_SimpleString("import sys; "
"print('sys.warnoptions:', sys.warnoptions); "
Expand Down Expand Up @@ -352,7 +375,7 @@ static int test_bpo20891(void)
return 1;
}

_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

unsigned long thrd = PyThread_start_new_thread(bpo20891_thread, &lock);
if (thrd == PYTHREAD_INVALID_THREAD_ID) {
Expand All @@ -375,7 +398,7 @@ static int test_bpo20891(void)

static int test_initialize_twice(void)
{
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

/* bpo-33932: Calling Py_Initialize() twice should do nothing
* (and not crash!). */
Expand All @@ -393,7 +416,7 @@ static int test_initialize_pymain(void)
L"print(f'Py_Main() after Py_Initialize: "
L"sys.argv={sys.argv}')"),
L"arg2"};
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

/* bpo-34008: Calling Py_Main() after Py_Initialize() must not crash */
Py_Main(Py_ARRAY_LENGTH(argv), argv);
Expand All @@ -416,7 +439,7 @@ dump_config(void)

static int test_init_initialize_config(void)
{
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
dump_config();
Py_Finalize();
return 0;
Expand Down Expand Up @@ -765,7 +788,7 @@ static int test_init_compat_env(void)
/* Test initialization from environment variables */
Py_IgnoreEnvironmentFlag = 0;
set_all_env_vars();
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
dump_config();
Py_Finalize();
return 0;
Expand Down Expand Up @@ -801,7 +824,7 @@ static int test_init_env_dev_mode(void)
/* Test initialization from environment variables */
Py_IgnoreEnvironmentFlag = 0;
set_all_env_vars_dev_mode();
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
dump_config();
Py_Finalize();
return 0;
Expand All @@ -814,7 +837,7 @@ static int test_init_env_dev_mode_alloc(void)
Py_IgnoreEnvironmentFlag = 0;
set_all_env_vars_dev_mode();
putenv("PYTHONMALLOC=malloc");
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
dump_config();
Py_Finalize();
return 0;
Expand Down Expand Up @@ -1154,7 +1177,7 @@ static int test_open_code_hook(void)
}

Py_IgnoreEnvironmentFlag = 0;
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
result = 0;

PyObject *r = PyFile_OpenCode("$$test-filename");
Expand Down Expand Up @@ -1218,7 +1241,7 @@ static int _test_audit(Py_ssize_t setValue)

Py_IgnoreEnvironmentFlag = 0;
PySys_AddAuditHook(_audit_hook, &sawSet);
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

if (PySys_Audit("_testembed.raise", NULL) == 0) {
printf("No error raised");
Expand Down Expand Up @@ -1274,7 +1297,7 @@ static int test_audit_subinterpreter(void)
{
Py_IgnoreEnvironmentFlag = 0;
PySys_AddAuditHook(_audit_subinterpreter_hook, NULL);
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

Py_NewInterpreter();
Py_NewInterpreter();
Expand Down Expand Up @@ -1869,13 +1892,13 @@ static int test_unicode_id_init(void)
_Py_IDENTIFIER(test_unicode_id_init);

// Initialize Python once without using the identifier
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
Py_Finalize();

// Now initialize Python multiple times and use the identifier.
// The first _PyUnicode_FromId() call initializes the identifier index.
for (int i=0; i<3; i++) {
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();

PyObject *str1, *str2;

Expand Down Expand Up @@ -2007,7 +2030,7 @@ unwrap_allocator(PyMemAllocatorEx *allocator)
static int
test_get_incomplete_frame(void)
{
_testembed_Py_Initialize();
_testembed_Py_InitializeFromConfig();
PyMemAllocatorEx allocator;
wrap_allocator(&allocator);
// Force an allocation with an incomplete (generator) frame:
Expand Down Expand Up @@ -2039,6 +2062,7 @@ struct TestCase
static struct TestCase TestCases[] = {
// Python initialization
{"test_repeated_init_exec", test_repeated_init_exec},
{"test_repeated_simple_init", test_repeated_simple_init},
{"test_forced_io_encoding", test_forced_io_encoding},
{"test_repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters},
{"test_repeated_init_and_inittab", test_repeated_init_and_inittab},
Expand Down
1 change: 1 addition & 0 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ Py_InitializeEx(int install_sigs)
config.install_signal_handlers = install_sigs;

status = Py_InitializeFromConfig(&config);
PyConfig_Clear(&config);
if (_PyStatus_EXCEPTION(status)) {
Py_ExitStatusException(status);
}
Expand Down

0 comments on commit c9b71f7

Please sign in to comment.