Importing external module (numpy) hangs up dolphin

[JimB16]

I'm not sure if I identified the exact problem so I will write down what I try to achieve.

I would like to use some modules that aren't in the embedded python of this project. For this I modify the sys.path and add the folder where I store the external modules and import numpy like this:

import sys
sys.path.append("path\\to\\Python\\Python38\\Lib\\site-packages")

import numpy as np

Loading this script the emulator hangs up, if I load the script directly in the command-line the Dolphin window doesn't even come up. I tried numpy versions 1.19.5 and then upgraded it and used 1.21.3, in both cases I had the same hang up.

Maybe that's just a problem on my end. Would be interested how you would import external modules that aren't included in the embedded python package.

[Felk]

You are doing exactly what I would have recommended to do to add libraries:

• install the same major version of python
• install libraries there
• manipulate the path to include your python installation's libraries

And in fact I am able to import other libraries this way, e.g. requests.

I can reproduce your issue. Looking at the stacktrace I can see that it blocks because initialization of numpy never returns. It deadlocks attempting to acquire the GIL:

python38.dll!_PyCOND_WAIT_MS(_PyCOND_T * cv, _RTL_CRITICAL_SECTION * cs, unsigned long ms) Line 171	C
python38.dll!PyCOND_TIMEDWAIT(_PyCOND_T *) Line 201	C
python38.dll!take_gil(_ceval_runtime_state * ceval, _ts * tstate) Line 206	C
python38.dll!PyEval_RestoreThread(_ts * tstate) Line 400	C
python38.dll!PyGILState_Ensure() Line 1305	C
...
python38.dll!_PyEval_EvalFrameDefault(_frame * f, int throwflag) Line 3500	C
...
python38.dll!PyImport_ImportModuleLevelObject(_object * name, _object * globals, _object * locals, _object * fromlist, int level) Line 1798	C
python38.dll!import_name(_ts * tstate, _frame * f, _object * name, _object * fromlist, _object * level) Line 5158	C
...
Dolphin.exe!Scripting::ScriptingBackend::ScriptingBackend(std::filesystem::path script_filepath) Line 21	C++
Dolphin.exe!ScriptsListModel::Add(std::string filename) Line 64	C++
Dolphin.exe!ScriptingWidget::AddNewScript() Line 79	C++
...
Dolphin.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * pCmdLine, int nCmdShow) Line 298	C++

I found this on a mailing list which looks like it describes the same issue: https://mail.python.org/pipermail/python-dev/2019-January/156095.html
This appears to be an issue with numpy, which does not support running in a python subinterpreter. I was unable to find any information on whether the situation has improved since, e.g. if updating the Python version would make a difference.

I am using subinterpreters to isolate concurrently running scripts, and to theoretically allow for multiple isolated dolphin instances within the same process. (Having no global state is a requirement if this fork ever wants to be merged upstream.)

I removed the usage of subinterpreters locally and was able to successfully import numpy. Assuming you don't actually need script-level isolation, please try this build and let me know if it works for you:
dolphin-scripting-without-python-subinterpreters.zip

If that works, it might be worthwhile to add the ability to disable subinterpreters via a config entry or something, so issues like these can be worked around.

Just for the record, this is what I did:

Index: Source/Core/Scripting/Python/PyScriptingBackend.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Source/Core/Scripting/Python/PyScriptingBackend.cpp b/Source/Core/Scripting/Python/PyScriptingBackend.cpp
--- a/Source/Core/Scripting/Python/PyScriptingBackend.cpp	(revision 46b7eacd5c810c2d21ec5fe51ea1a9c61a7ceb3d)
+++ b/Source/Core/Scripting/Python/PyScriptingBackend.cpp	(date 1636055002326)
@@ -99,14 +99,6 @@
   }
 }
 
-static void ShutdownMainPythonInterpreter()
-{
-  if (Py_FinalizeEx() != 0)
-  {
-    ERROR_LOG(SCRIPTING, "Unexpectedly failed to finalize python");
-  }
-}
-
 PyScriptingBackend::PyScriptingBackend(std::filesystem::path script_filepath,
                                        API::EventHub& event_hub, API::Gui& gui,
                                        API::GCManip& gc_manip, API::WiiManip& wii_manip)
@@ -118,11 +110,9 @@
     s_main_threadstate = InitMainPythonInterpreter();
   }
   PyEval_RestoreThread(s_main_threadstate);
-  m_interp_threadstate = Py_NewInterpreter();
-  PyThreadState_Swap(m_interp_threadstate);
-  u64 interp_id = PyInterpreterState_GetID(m_interp_threadstate->interp);
-  s_instances[interp_id] = this;
+  u64 interp_id = PyInterpreterState_GetID(s_main_threadstate->interp);
 
+  if (s_instances.empty())
   {
     // new scope because we need to drop these PyObjects before we release the GIL
     // below (PyEval_SaveThread) because DECREF-ing them needs the GIL to be held.
@@ -139,6 +129,7 @@
       PyErr_Print();
     }
   }
+  s_instances[interp_id] = this;
 
   Init(script_filepath);
 
@@ -147,23 +138,6 @@
 
 PyScriptingBackend::~PyScriptingBackend()
 {
-  std::lock_guard lock{s_bookkeeping_lock};
-  if (m_interp_threadstate == nullptr)
-    return;  // we've been moved from (if moving was implemented)
-  PyEval_RestoreThread(m_interp_threadstate);
-  u64 interp_id = PyInterpreterState_GetID(m_interp_threadstate->interp);
-  s_instances.erase(interp_id);
-  Py_EndInterpreter(m_interp_threadstate);
-  PyThreadState_Swap(s_main_threadstate);
-  if (s_instances.empty())
-  {
-    ShutdownMainPythonInterpreter();
-    s_main_threadstate = nullptr;
-  }
-  else
-  {
-    PyEval_SaveThread();
-  }
 }
 
 // Each PyScriptingBackend manages one python sub-interpreter.

[JimB16]

Thanks for the quick fix, the dolphin you linked here worked for me. And it seems like all the features that I needed to test my project idea are working in this version.

[Felk]

I added the command line option --no-python-subinterpreters in 45fa32f which properly implements the workaround described above. However, I'll keep this issue open because it's still just a workaround I suppose.

[14sirs]

I added the command line option --no-python-subinterpreters in 45fa32f which properly implements the workaround described above. However, I'll keep this issue open because it's still just a workaround I suppose.

I have tried using this command line option, but Dolphin still doesn't like it when I try to import numpy - do you know why?

[Felk]

I suppose it's worth to try out embedding a gil-less python (free-threaded) into dolphin, given numpy apparently hangs trying to acquire the GIL (at least in the original scenario). Maybe all these issues then vanish, given the folks over at numpy seem to have been hard at work to make numpy support gil-less python: numpy/numpy#26157

[14sirs]

I suppose it's worth to try out embedding a gil-less python (free-threaded) into dolphin, given numpy apparently hangs trying to acquire the GIL (at least in the original scenario). Maybe all these issues then vanish, given the folks over at numpy seem to have been hard at work to make numpy support gil-less python: numpy/numpy#26157

@Felk You say to embed it in dolphin, where in dolphin does it need to be embedded? is it in "\Binary\x64\python-embed\python312" or does it need to be embedded elsewhere?

[Felk]

Sorry, that was more of a note to myself. Updating the python version that ships with this repository's custom build of dolphin requires actual programming work by me or anyone familiar with the codebase and python