Skip to content

Calling python function from thread created in C++ #1723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
banasraf opened this issue Mar 11, 2019 · 7 comments
Closed

Calling python function from thread created in C++ #1723

banasraf opened this issue Mar 11, 2019 · 7 comments

Comments

@banasraf
Copy link

I encountered a problem when trying to implement functionality similar to that on the snippet.

#include <pybind11/pybind11.h>
#include <iostream>
#include <thread>
#include <future>

namespace py = pybind11;

void hello() {
  std::cout << "hello\n";
}

struct Executor {

  void execute() {
    auto func = channel.get_future().get();
    func();
  }

  explicit Executor(py::object callback): callback(std::move(callback)), channel(), worker([this]{execute();}) {
  }

  void run() {
    pybind11::gil_scoped_release release{};
    channel.set_value(callback);
  }

  py::object callback;
  std::promise<py::object> channel;
  std::thread worker;

  ~Executor() {
    worker.join();
  }
};

PYBIND11_MODULE(pymodule, m) {
  m.def("hello", &hello, "Say hello");
  py::class_<Executor>(m, "Executor")
      .def(py::init<py::object>())
      .def("run", &Executor::run);
}

testing it with this python code:

import pymodule

def bye():
    print('bye')

if __name__ == '__main__':
    pymodule.hello()
    ex = pymodule.Executor(bye)
    ex.run()

The behavior is nondeterministic. For some runs it goes without any error and sometimes it ends with SEGFAULT. I saw similar issues here but none of the solutions worked for me. I tried to put gil_scoped_acquire and gil_scoped_release in every possible combination but in the result the python function was not called without any error, or just segmentation fault happened.

@eacousineau
Copy link
Contributor

You're crossing over into GIL (Global Interpreter Lock) territory - good luck!
https://wiki.python.org/moin/GlobalInterpreterLock

If your C++ bindings are relatively small, then you should play around with py::gil_scoped_acquire and py::gil_scoped_release mechanisms:
https://pybind11.readthedocs.io/en/stable/advanced/misc.html#gil
https://pybind11.readthedocs.io/en/stable/advanced/functions.html#call-guard

If your bindings are rather large and comprehensive, I'd suggest trying to think of alternative ways of doing things... Perhaps Python's mulitprocessing or something else.
(Full GIL compatibility may be achievable, but you may have to py::call_guard most of your bindings...)

@banasraf
Copy link
Author

Any tips why with such gil guard:

void run() {
    py::gil_scoped_release guard{};
    channel.set_value(callback);
 }

void execute() {
    py::gil_scoped_acquire guard{};
    auto func = channel.get_future().get();
    func();
 }

the script doesn't deadlock nor throws any error, but simply doesn't call the passed function and exits silently?

@dpellegr
Copy link

Could #1273 help?

@eacousineau
Copy link
Contributor

Also, does your repro versino incorporating #1211 in pybind11?

@eacousineau
Copy link
Contributor

Er, I don't know exactly why, but the following (I think?) fixes your code (on my machine for my setup...):
eacousineau/repro@e7b199c

Some wild speculation / suggestions:

  • Try to keep C++ objects as C++ objects. You were using std::future<py::object> - this does ref incr + decr automatically, which requires GIL, so you may have had GIL sync issues. In this case, std::function<void ()> serves the purpose of keeping C++ objects the same - see below about wrappage.
    • There may still be lifetime issues when deallocating the std::function<> that wraps the callable... @jagerman Do you (or someone else) knows what the correct flow is, and if there is anything that needs to be properly guarded against?
  • You were doing gil_scoped_release when queuing up the function which... doesn't make sense, b/c the function is queued when the GIL is held anywho?
    • In this case, I think you wanted to acquire the GIL when executing the function. Added some stuff on that.

I added a comment there about maybe making some wrapper code that will auto-GIL acquire for any std::function<> / py::function arguments.

If you wanna see some wacky but generic argument wrapper code stuff, here you go:
https://github.com/RobotLocomotion/drake/blob/b9bd9921aaf99dc9123aaf34cc1f62fe2c34ac55/bindings/pydrake/common/wrap_pybind.h#L71-L83

@banasraf
Copy link
Author

I fixed that by releasing the gil in a proper place. I added a method:

void sync() {
   py::gil_scoped_release guard{};
   worker.join()
}

For some reason doing this in the Executor's destructor didn't work.

@MadangLightHouse
Copy link

Hi,
For some reason doing this in the Executor's destructor didn't work.

Wouldn't that have just been because the call to Executor's destructor is non-deterministic (since it is emitted by the Python garbage collector)?

Just asking?

Cheers & thanks for the examples & solutions. I'm coming from the C++ world & only just beginning to feel my way into the world of Python via pybind11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants