View Issue Details

IDProjectCategoryView StatusLast Update
0005469DarkRadiantDesign/Codingpublic17.01.2021 06:02
Reporterjonri Assigned Togreebo  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
PlatformArch Linux 
Product Version2.10.0 
Target Version2.11.0Fixed in Version2.11.0 
Summary0005469: Crash in master on startup
DescriptionThe 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 [1], 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.


[1] : https://github.com/codereader/DarkRadiant/commit/4a643a91fcbbd4bdb7c451e31b96d42130f515a7#diff-833f183745cc2c5e7d8bba4fde1897e7461887deee90f9a5f0a47ef05b859c37R18
Additional InformationI'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 :-)
TagsNo tags attached.

Relationships

has duplicate 0005490 resolvedorbweaver darkradiant 2.10.0 crashes in pybind / python scripting interface 

Activities

greebo

greebo

28.12.2020 17:13

administrator   ~0013302

Just pushed a fix. Is it working better now? In my Arch VM this is not crashing, neither at startup nor shutdown.
jonri

jonri

28.12.2020 18:09

developer   ~0013303

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.
greebo

greebo

28.12.2020 18:11

administrator   ~0013304

Nice, thanks for the confirmation and the research.

Related Changesets

DarkRadiant: master 6b10c158

2020-12-28 17:12:39

greebo

Details Diff
0005469: Fix crash due to pybind11::dict being initialised too early. Affected Issues
0005469
mod - plugins/script/PythonModule.cpp Diff File
mod - plugins/script/PythonModule.h Diff File

Issue History

Date Modified Username Field Change
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