View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005469||DarkRadiant||Design/Coding||public||28.12.2020 04:42||17.01.2021 06:02|
|Target Version||2.11.0||Fixed in Version||2.11.0|
|Summary||0005469: Crash in master on startup|
|Description||The latest master crashes on startup every time. I've tracked it down to the PythonModule constructor, specifically when initializing the `py::dict _globals` member.|
Until the recent refactor , this used to be a `std::unique_ptr<py::dict>` which was lazy-initialized in `getGlobals()`. I suspect that with it now being a member, pybind11 doesn't like trying to initialize a py::dict that early, and some other python initialization is supposed to take place first.
I reverted _globals to use a unique pointer that is lazy-initialized in getGlobals(), and I was then able to start up DarkRadiant. Once it started up I tested a python script and it also worked.
I commented out the cleanup code using _globals since I wasn't sure off the top of my head how it would interact with the pointer, and that caused a crash on exit. So, if reverting to a pointer is the fix for this, that will have to be dealt with properly too.
As a side note, `py::module _module` is another member of PythonModule which also used to be a pointer. It doesn't seem to be causing a problem for me but I'm not sure if it is correct either.
I'll defer to your expertise on fixing this, I'd think going back to a pointer would be fine but we'd want to logically ensure it doesn't get initialized before it's supposed to.
 : https://github.com/codereader/DarkRadiant/commit/4a643a91fcbbd4bdb7c451e31b96d42130f515a7#diff-833f183745cc2c5e7d8bba4fde1897e7461887deee90f9a5f0a47ef05b859c37R18
|Additional Information||I'm still setting up a Windows build environment for comparison but I'm out of time today. My main guess as to why this went uncaught is that Arch tends to run more recent versions of python (3.9.1) which might be more sensitive to the problem. The good news is this means we can solve these kinds of problems before they have a chance to exist on more popular distros like Ubuntu :-)|
|Tags||No tags attached.|
|Just pushed a fix. Is it working better now? In my Arch VM this is not crashing, neither at startup nor shutdown.|
Yes, I confirmed this fixed it for me.
I found the specific "something" that wasn't happening in time. The `initialize_interpreter()` pybind11 function has this comment:
Initialize the Python interpreter. No other pybind11 or CPython API functions can be
called before this is done; with the exception of `PYBIND11_EMBEDDED_MODULE`.
We don't call `initialize_interpreter()` until `PythonModule::initialise()` gets called, so anything that calls into python before that point (i.e. the py::dict constructor) is prone to failure. I'm guessing the py::module member is fine to leave alone since they left an exception for the PYBIND11_EMBEDDED_MODULE macro.
I think we can put this one to bed.
|Nice, thanks for the confirmation and the research.|
|28.12.2020 04:42||jonri||New Issue|
|28.12.2020 04:42||jonri||Status||new => assigned|
|28.12.2020 04:42||jonri||Assigned To||=> greebo|
|28.12.2020 04:43||jonri||Description Updated||View Revisions|
|28.12.2020 17:12||greebo||Changeset attached||=> DarkRadiant master 6b10c158|
|28.12.2020 17:13||greebo||Note Added: 0013302|
|28.12.2020 18:09||jonri||Note Added: 0013303|
|28.12.2020 18:11||greebo||Note Added: 0013304|
|28.12.2020 18:12||greebo||Target Version||=> 2.11.0|
|28.12.2020 18:12||greebo||Status||assigned => resolved|
|28.12.2020 18:12||greebo||Resolution||open => fixed|
|28.12.2020 18:12||greebo||Fixed in Version||=> 2.11.0|
|17.01.2021 06:00||greebo||Relationship added||related to 0005490|
|17.01.2021 06:02||greebo||Relationship replaced||has duplicate 0005490|