[BUG]: Cannot import pybind11-based python module from subinterpreter

[mdorier]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• Consider asking first in the Gitter chat room or in a Discussion.

Problem description

I have a code that uses Python's C API (not pybind11) and creates subinterpreters in which to run code. The code works fine, apart from when a subinterpreter tries to import a python package that is based on pybind11: in such a situation, it hangs. Importing other, non-pybind11 packages, works fine.

Reproducible example code

(main.cpp) Code that runs subinterpreters with Python C API:

#include <thread>
#include <Python.h>

// initialize and clean up python
struct initialize {
    initialize() {
        Py_InitializeEx(1);
        PyEval_InitThreads();
    }

    ~initialize() {
        Py_Finalize();
    }
};

// allow other threads to run
class enable_threads {
public:
    enable_threads() {
        _state = PyEval_SaveThread();
    }

    ~enable_threads() {
        PyEval_RestoreThread(_state);
    }

private:
    PyThreadState* _state;
};

// acquire and release the GIL
struct gil_lock
{
    PyGILState_STATE gstate;

    gil_lock() {
        gstate = PyGILState_Ensure();
    }

    ~gil_lock() {
        PyGILState_Release(gstate);
    }
};

// restore the thread state when the object goes out of scope
class restore_tstate
{
public:

    restore_tstate() {
        _ts = PyThreadState_Get();
    }

    ~restore_tstate() {
        PyThreadState_Swap(_ts);
    }

private:
    PyThreadState* _ts;
};

// swap the current thread state with ts, restore when the object goes out of scope
class swap_tstate
{
public:

    swap_tstate(PyThreadState* ts) {
        _ts = PyThreadState_Swap(ts);
    }

    ~swap_tstate() {
        PyThreadState_Swap(_ts);
    }

private:
    PyThreadState* _ts;
};

// create new thread state for interpreter interp, clean up on destruction
class thread_state
{
public:

    thread_state(PyInterpreterState* interp) {
        _ts = PyThreadState_New(interp);
    }

    ~thread_state() {
        if( _ts ) {
            PyThreadState_Clear(_ts);
            PyThreadState_Delete(_ts);

            _ts = nullptr;
        }
    }

    operator PyThreadState*() {
        return _ts;
    }

private:
    PyThreadState* _ts;
};

// represent a sub interpreter
class sub_interpreter
{
public:

    // perform the necessary setup and cleanup for a new thread running using a specific interpreter
    struct thread {
        gil_lock _lock;
        thread_state _state;
        swap_tstate _swap;

        thread(PyInterpreterState* interp) :
                _state(interp),
                _swap(_state)
        {}
    };

    sub_interpreter() {
        restore_tstate restore;
        _ts = Py_NewInterpreter();
    }

    ~sub_interpreter() {
        if( _ts ) {
            swap_tstate sts(_ts);
            Py_EndInterpreter(_ts);
        }
    }

    PyInterpreterState* interp() {
        return _ts->interp;
    }

private:
    PyThreadState* _ts;
};

char program[] = "print(\"Subinterpreter\"); import sys; sys.path.append('.'); import my_module";
char main_program[] = "print(\"Main program\")";

// run in a new thread
void f(PyInterpreterState* interp)
{
    sub_interpreter::thread scope(interp);
    PyRun_SimpleString(program);
}

int main()
{
    initialize init;

    sub_interpreter s1;
    sub_interpreter s2;
    sub_interpreter s3;

    std::thread t1(f, s1.interp() );
    std::thread t2(f, s2.interp() );
    std::thread t3(f, s3.interp() );

    std::thread t4(f, s1.interp() );

    PyRun_SimpleString(main_program);

    enable_threads t;

    t1.join();
    t2.join();
    t3.join();
    t4.join();

    return 0;
}

(module.cpp) Code of a pybind11-based module

#include <pybind11/pybind11.h>

namespace py = pybind11;

PYBIND11_MODULE(my_module, m) {
    m.doc() = "Example module";
};

(CMakeLists.txt) CMake file to build the project

cmake_minimum_required (VERSION 3.15)

project (py1)

find_package (Python3 COMPONENTS Interpreter Development REQUIRED)
find_package (pybind11 REQUIRED)
include_directories (${PYTHON_INCLUDE_DIRS})

ADD_DEFINITIONS (-std=c++11)

add_executable (main main.cpp)
target_link_libraries (main Python3::Python)

add_library (my_module MODULE module.cpp)
target_link_libraries (my_module PRIVATE pybind11::module)
pybind11_extension (my_module)
pybind11_strip (my_module)

[EthanSteinberg]

I can confirm that I can reproduce this bug, investigating now.

[EthanSteinberg]

The cause of the problem is that pybind11 is using PyGILState_Ensure and PyGILState_Release, which explicitly doesn't support sub interpreters.

We would need to migrate pybind11 over to PyEval_SaveThread and PyEval_RestoreThread in order to add sub interpreter support.

[Skylion007]

@rwgk This may affect your SimpleGIL refactor.

[rwgk]

At first glance: none of the 7 RAII types in the reproducer follows the rule-of-3 (or rule-of-5), which is bug prone. @mdorier did you rule out already that that's not an issue (in the reproducer or the original code)? The first thing I'd do is make those types safe. Even if it turns out that there was no problem, sooner or later someone will fall off a cliff because of the missing guard rails, especially when debugging.

[EthanSteinberg]

@rwgk It doesn't matter if his code is buggy or not since we are using an API which itself declares in the Python docs that it shouldn't work with sub-interpreters.

Note that the PyGILState_* functions assume there is only one global interpreter (created automatically by Py_Initialize()). Python supports the creation of additional interpreters (using Py_NewInterpreter()), but mixing multiple interpreters and the PyGILState_* API is unsupported.

https://docs.python.org/3/c-api/init.html#non-python-created-threads

[mdorier]

I copied this code from an example found online about how to use subinterpreters. This is not the code in my codebase (my code base is in C, doing "manually" the kind of locking/unlocking that the code above dose with RAII). It was just simpler for a reproducer. However just looking at it, none of the variables involved are moved or copied, so I'm pretty sure this is not the problem.

[rwgk]

@rwgk It doesn't matter if his code is buggy or not since we are using an API which itself declares in the Python docs that it shouldn't work with sub-interpreters.

Note that the PyGILState_* functions assume there is only one global interpreter (created automatically by Py_Initialize()). Python supports the creation of additional interpreters (using Py_NewInterpreter()), but mixing multiple interpreters and the PyGILState_* API is unsupported.

https://docs.python.org/3/c-api/init.html#non-python-created-threads

Thanks for the pointer! I'm totally not familiar with this.

IIUC that basically makes this bug a feature request.

Sorry small rant, to explain where I'm coming from, my personal opinion not reflecting anything official: I believe threads are one of the worst aberrations of software engineering, and subinterpreters, too, creating more problems than they solve. Like a junkie finds joy in a shot even though it is destroying his life, there seems to be a lot of joy in short-sighted optimizations.

With that said: If we want to support subinterpreters, fine by me, but it looks like someone will need to sink in a significant amount of time, especially because we'd need full testing: anything that's not tested is likely to be broken one way or another sooner or later. Full testing looks tricky: highly platform dependent, the combination of fork, threads, subinterpreters is a bit of a combinatorial explosion of situations to test.

[rwgk]

It would be much better to tell people pybind11_fail("sorry, subinterpreters are not supported"); than to let me fall into a trap (e.g. not knowing why their code hangs). Is that feasible? Maybe only in debug mode, in case there is runtime overhead for checking.

[EthanSteinberg]

Yeah, this is pretty much a feature request and feels low priority given that sub-interpreters are a pretty niche feature.

It would be much better to tell people pybind11_fail("sorry, subinterpreters are not supported"); than to let me fall into a trap (e.g. not knowing why their code hangs). Is that feasible?

In theory this should be trivial. assert(PyInterpreterState_Main() == PyThreadState_GetInterpreter()). I might try a simple PR this weekend.

[rwgk]

In theory this should be trivial. assert(PyInterpreterState_Main() == PyThreadState_GetInterpreter()). I might try a simple PR this weekend.

Awesome, thanks!

[Tabrizian]

@rwgk Python 3.12 has implemented PEP 684 which allows each sub-interpreter to have its own GIL. Does this move the needle for supporting sub-interpreter interface in pybind11?

[rwgk]

Does this move the needle for supporting sub-interpreter interface in pybind11?

What exactly do you have in mind?

From my end:

• If someone sends a PR complete with unit tests, I'll review as best I can.

Side remarks:

• I still don't have any experience working with sub-interpreters.

• If someone spends the time putting together a high-quality PR, that's a signal for me: it has become important enough to spend my time learning about it. :-)

• It seems very unlikely to me that any of the pybind11 maintainers will take on actively working on such a PR any time soon.

[Tabrizian]

What exactly do you have in mind?

I meant mostly providing a path for embedding sub-interpreters in pybind11 without having to use the underlying Python C-API.

For example, it could be another RAII interface (e.g., py::scoped_subinterpreter) with methods that allow releasing or acquiring the GIL associated with each sub interpreter.

[rwgk]

For example, it could be another RAII interface (e.g., py::scoped_subinterpreter) with methods that allow releasing or acquiring the GIL associated with each sub interpreter.

Sounds good.