pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.65k stars 2.1k forks source link

gil_scoped_acquire deadlock #1273

Open KyleFromKitware opened 6 years ago

KyleFromKitware commented 6 years ago

Let's say we have the following minimal example:

#include <pybind11/pybind11.h>
#include <pybind11/embed.h>

#include <iostream>
#include <thread>

static void thread()
{
  pybind11::gil_scoped_acquire acquire;
  std::cout << "Calling Python code from new thread" << std::endl;
}

int main()
{
  pybind11::scoped_interpreter interp;

  {
    pybind11::gil_scoped_acquire acquire;
    std::cout << "Calling Python code from main thread" << std::endl;
  }

  std::thread(thread).join();

  return 0;
}

We would expect this to print:

Calling Python code from main thread
Calling Python code from new thread

But the cout from the new thread is never reached because of a GIL deadlock. The reason this is happening is because PyEval_InitThreads(), in addition to initializing threads, also acquires the GIL and never releases it. gil_scoped_acquire calls PyEval_InitThreads() the first time it runs, and it acquires the GIL a second time, so even when gil_scoped_acquire is destructed, the GIL is still held.

If we change the example to this:

#include <pybind11/pybind11.h>
#include <pybind11/embed.h>

#include <iostream>
#include <thread>

static void thread()
{
  pybind11::gil_scoped_acquire acquire;
  std::cout << "Calling Python code from new thread" << std::endl;
}

static void init_threads()
{
  if (!PyEval_ThreadsInitialized())
  {
    {
      pybind11::gil_scoped_acquire acquire;
    }
    PyEval_SaveThread();
  }
}

int main()
{
  pybind11::scoped_interpreter interp;

  init_threads();

  {
    pybind11::gil_scoped_acquire acquire;
    std::cout << "Calling Python code from main thread" << std::endl;
  }

  std::thread(thread).join();

  return 0;
}

then everything behaves as expected. Is this what I'm supposed to do, or is this a bug in pybind?

jagerman commented 6 years ago

scoped_interpreter holds a GIL by design, but gil_scoped_release is meant to let you release it (i.e. when you know it is safe to do so). This, then, should work:

#include <pybind11/pybind11.h>
#include <pybind11/embed.h>

#include <iostream>
#include <thread>

static void thread()
{
  pybind11::gil_scoped_acquire acquire;
  std::cout << "Calling Python code from new thread" << std::endl;
}

int main()
{
  pybind11::scoped_interpreter interp;

  {
    pybind11::gil_scoped_acquire acquire;
    std::cout << "Calling Python code from main thread" << std::endl;
  }

  {
    pybind11::gil_scoped_release release;
    std::thread(thread).join();
  }

  return 0;
}
eacousineau commented 5 years ago

Just to check: Is this resolved by https://github.com/pybind/pybind11/pull/1211?