PyInstaller is a tool for freezing Python applications into stand-alone packages, much like py2exe. py2app, and bbfreeze. PyInstaller is providing *one* bootloader for all versions of Python supported (2.7, 3.4-3.6).

In PyInstaller the startup sequence is implemented in
pyi_pylib_start_python() in bootloader/src/pyi_pythonlib.c. The workflow roughly is:

• SetProgramName
• SetPythonHome
• Py_SetPath
• Setting runtime options
  • some flags using the global variables
  • PySys_AddWarnOption -> crash
• Py_Initialize
• PySys_SetPath

The crash occurs due to tstate (thread state) not being initialized when
calling PySys_AddWarnOption.

I don't think we made any start-up changes that were specific to PySys_AddWarnOption, so my suspicion is that the crash is going to be related to a change in the constraints on either the unicode object creation or the list append operation.

The attached patch adds a new test case (I also cleaned up several other details related to test_embed execution in order to more easily see where the segfault happens on failure).

Unfortunately, it isn't yet suitable for use with git bisect, as I checked 3.7.0a3 (the earliest tag where the patch applied cleanly), and that commit also segfaults. So setting up for git bisect testing (as per the hot-fix example in https://git-scm.com/docs/git-bisect#_examples ) will require:

• checking the commit where "test_embed.py" was extracted from "test_capi.py" and seeing whether or not that segfaults
• if it *doesn't* segfault, git bisect between there and v3.7.0a3
• if it *does* segfault, make a revised test patch that applies cleanly to early versions, and then go back through the 3.7.0 pre-releases to find one that works

Python documentation was enhanced in Python 3.7 to explicitly list all functions safe to call *before* Py_Initialize():

https://docs.python.org/dev/c-api/init.html#before-python-initialization

PySys_AddWarnOption() is not part of the list. While it's not in the list, I'm kind of unhappy that we broke your use case: it wasn't my intent. Because I broken your use case with this change part of the big bpo-32030 refactoring:

commit f7e5b56
Author: Victor Stinner <victor.stinner@gmail.com>
Date: Wed Nov 15 15:48:08 2017 -0800

bpo-32030: Split Py_Main() into subfunctions (bpo-4399)

IHMO the regression is that PySys_AddWarnOption() now calls _PySys_GetObjectId(): in Python 3.6, it wasn't the case.

Python 3.6 code:
---

void
PySys_AddWarnOptionUnicode(PyObject *unicode)
{
    if (warnoptions == NULL || !PyList_Check(warnoptions)) {
        Py_XDECREF(warnoptions);
        warnoptions = PyList_New(0);
        if (warnoptions == NULL)
            return;
    }
    PyList_Append(warnoptions, unicode);
}

Again, it's a bad idea to use the Python API before Py_Initialize(): you likely have to build a Unicode string and you use a list, whereas these two object types are not properly initialized...

The PEP-432 and bpo-32030 prepared Python to have a much better API in Python 3.8 for embedding Python. You will be able to use a wchar_t* string to pass warning options.

Adding a bit of context from my prior email discussion with Hartmut: CPython actually reads sys.warnoptions at the end of Py_Initialize (its the last thing we do before the function returns).

It also reads sys._xoptions during startup, since that's one way of enabling settings like dev mode and UTF-8 mode.

That means that even though PySys_AddWarnOption and PySys_AddXOption weren't documented as being safe to call before Py_Initialize, they likely *won't* have the desired effect if an embedding application defers calling them until later, and their absence from the list of "safe to call before Py_Initialize" functions was itself a documentation bug.

For 3.8, I'd hoped that the resolution might be as simple as a hard requirement on embedding applications to call PyRuntime_Initialize() before doing *anything* with the C API, but including the "internal/pystate.h" header and adding a call to _PyRuntime_Initialize() wasn't enough to keep the draft test case in my patch from segfaulting :(

b4d1e1f (the first attempted fix for bpo-20891) is the last commit where the draft test case patch applies cleanly.

That still segfaults, so I'll jump all the way back to 3.7.0a1, rework the patch to apply there, and see if pre-init use of these APIs were still working back then.

After reworking the patch to backport the pre-initialization embedding tests to 3.7.0a1, they *both* failed, as that was before Victor fixed Py_DecodeLocale to work prior to initialization again.

As a result, I tried 43605e6 in November, after the initial merge of the PEP-432 working branch, but just before the start of Victor's follow on refactoring to really take advantage of the new internal structures: double failure there as well.

f5ea83f is the next point I checked (just before Eric's September changes to global state management), and it looks like we have a new chief suspect, as applying the attached "earlier-tree" version of the patch at this point in the history gives two passing test cases :)

Next I'll work out a git bisect run based on those two start and end commits and the "earlier-tree" patch, and attempt to narrow the failure down to a specific commit (probably tomorrow).

(Even without that bisect run though, the warnoptions changes in 2ebc5ce#diff-f38879f4833a6b6847e556b9a07bf4ed are now looking *very* suspicious)

I suggest to revert my changes on PySys_AddWarnOption*() and "just" re-use the code from Python 3.6.

Previously, I made the code more complex since I had to expose a private function to add manually warning options. But later, the code changed again, so the private function is gone.

While I haven't definitely narrowed it down yet, I suspect it isn't your changes that are the problem: since the reported crash relates to attempting to access a not-yet-created thread state, warnoptions and _xoptions likely need to become C level globals again for 3.7 (along with anything needed to create list and Unicode objects).

We can then transfer them from there into Eric's new runtime state objects in _PyRuntime_Initialize (or potentially _Py_InitializeCore).

Patched test_capi results for 2ebc5ce (the global state consolidation commit): both pre-initialization tests fail

Patched test_capi results for bab21fa (the immediately preceding commit): both pre-initialization tests pass

Ding, ding, ding, we have a winner :)

Rather than reverting the options-related aspects of that change though, I think a nicer way to handle it will be to add a "pre-init fallback" path to those functions that stores the raw wchar_t data, and postpones converting those values to Py_Unicode objects until we need them in _PyInitialize_Core.

I'm not sure what to do about PySys_AddWarnOptionUnicode though: I think that's just outright incoherent as an API, since you need to call it before Py_Initialize for it to do anything useful, but we don't support calling any of the Unicode creation APIs until *after* Py_Initialize.

Nick, what's your assessment of this issue's priority? Is it a release blocker for 3.7.0b3 (proposed ABI freeze)? For 3.7.0rc1 (code freeze)? None of the above?

This shouldn't affect the public ABI, so I don't think it's a blocker for the ABI freeze, but it's a regression that effectively makes PySys_AddWarnOption (and related APIs) unusable, so we should definitely fix it before shipping 3.7.

For now, I've marked the bug as critical, and I'll be aiming to have it fixed before 3.7.0b3 at the end of the month.

For PySys_AddWarnOption and PySys_ResetWarnOptions, my current plan is to adjust them to maintain a singly-linked list of "wchar *" pointers when called prior to _Py_InitializeCore (i.e. when "PyThreadState_GET() == NULL"), while using the existing 3.7.0b2 path when the thread state is available. _Py_InitializeCore will then take care of cleaning up the memory allocations while converting them to Python unicode entries in a Python list object.

_PySys_AddXOption would work similarly (just with a separate list).

For PySys_AddWarnOptionUnicode, I think we need to make it call Py_FatalError for 3.7 when called prior to Py_Initialize, with a recommendation to use PySys_AddWarnOption instead. In 3.8, that API will become useful again, once the public Py_InitializeCore API provides a supported multi-stage startup process that allows unicode objects to be created safely before Py_InitializeMainInterpreter (or whatever we end up calling that API) reads sys.warnoptions and configures the warning subsystem accordingly.

The linked PR has the draft test case for this - it goes beyond the ones I used to find the cause of the problem by actually checking that sys.warnoptions and sys._xoptions have been populated as expected.

Thinking about PySys_AddWarnOptionUnicode a little further, I'm not sure we actually need to change anything in the implementation there - I think we can just make it clearer in the docs that *only* PySys_AddWarnOption is supported prior to Py_Initialize for the embedding use case - the unicode version of the API currently only makes sense in CPython, where we can call it after _Py_InitializeCore.

It will make sense for embedding applications in 3.8 once we make Py_InitializeCore a public API.

PR has been updated to be mostly complete (just pending docs changes now), but I think I've found a potential issue with the interaction between the way I've currently implemented it and the way _Py_InitializeCore and _Py_InitializeMainInterpreter work.

Specifically, sys.warnoptions and sys._xoptions don't get created until _PySys_EndInit, so that's where I added the code to read the pre-initialization linked lists and move those values into sys.warnoptions and sys._xoptions. The current test is just checking that those sys attribute have the expected contents - it isn't checking that the consequences of those settings have correctly propagated to the warnings filter list.

For the default filters add by _PyWarnings_Init at the end of _Py_InitializeCore, I think that's fine - we're going to want the embedding application's filters to be add after the default filter list anyway.

However, the code in _PyInitialize_MainInterpreter to actually import the warnings module (which then reads sys.warnoptions) is guarded by a check for a non-empty config->warnoptions, and that's not going to trip in the case where get_warnoptions() has created a new sys.warnoptions list, and config->warnoptions is still NULL.

Rather than changing the preinit sys module code to be config-aware, I'm thinking that what I'd like to do is:

1. Update the new test case to also check that the most recent 3 entries in the warnings filter list match what we expect
2. Assuming that fails (as I expect it will), change the guard in _Py_InitializeMainInterpreter to check PySys_HasWarnOptions (which will correctly handle the case where config->warnoptions is NULL, but entries have been added to sys.warnoptions by some other mechanism, like PySys_AddWarnOption)

I added the new test case, and found to my surprise that it didn't fail, even in debug mode (where there aren't any default filters).

The point I had missed was that even though warnoptions can be NULL in CoreConfig, _Py_MainInterpreterConfig_Read ensures that it's never NULL for the main interpreter config (which is what _PySys_EndInit reads), which means that neither we nor any embedding application can currently trigger the code path in get_warnoptions() that lazily creates the warnings list without adding the reference back into the config settings.

We'll need to fix that before we make the new configuration API public, but for now I'm just going to note it in the source code as a "PEP-432 TODO".

New changeset c6d94c3 by Miss Islington (bot) in branch '3.7':
bpo-33042: Fix pre-initialization sys module configuration (GH-6157)
c6d94c3

While I've merged an initial fix for this (so it should be working again in 3.7.0b3), I'm not going to close the issue until folks have had a chance to review and comment on the linked list based approach I've taken.

New changeset bc77eff by Nick Coghlan in branch 'master':
bpo-33042: Fix pre-initialization sys module configuration (GH-6157)
bc77eff

Would it be possible to not officially allow calling PySys_AddWarnOption() before Py_Initialize()? Fix the bug, but don't promote doing that. I just suggest to revert:

Doc/c-api/init.rst:

+ * :c:func:`PySys_AddWarnOption`
+ * :c:func:`PySys_AddXOption`
+ * :c:func:`PySys_ResetWarnOptions`

I can confirm that c6d94c3 makes the PyInstaller test-case, which war the origin of this bug-report, pass.

Thanks for fixing!

Harmut, thanks for the status update!

Victor, the problem is that both sys.warnoptions and sys._xoptions get read by Py_Initialize(), so setting them afterwards is essentially pointless - while you do still change the contents of the sys attributes, doing so doesn't have any further effect.

In 3.8, we'll hopefully make an updated version of PEP-432 public, and hence be able to deprecate calling them prior to Py_InitializeCore, and either advise folks to use the the new config structs instead, or else adjust the way that _PySys_BeginInit and _PySys_EndInit work so that the relevant sys module attributes get created in Py_InitializeCore rather than in Py_InitializeMainInterpreter.

Just noting that while I've closed this issue as fixed, I think we still have quite a bit of work to do to get our handling of these pre-init config settings to a place we're genuinely happy with (or as happy as backwards compatibility constraints permit).

However, the next step in that process is likely to involve at least myself, Victor & Eric getting together in person to discuss it at PyCon US (as I'm pretty sure we'll all be there this year), and deciding on how we'd like PEP-432 to look for Python 3.8.