6

I designed a C++ system that invokes user defined callbacks from procedure running in a separate thread. Simplified system.hpp looks like this:

#pragma once

#include <atomic>
#include <chrono>
#include <functional>
#include <thread>

class System
{
public:
  using Callback = std::function<void(int)>;
  System(): t_(), cb_(), stop_(true) {}
  ~System()
  {
    stop();
  }
  bool start()
  {
    if (t_.joinable()) return false;
    stop_ = false;
    t_ = std::thread([this]()
    {
      while (!stop_)
      {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
        if (cb_) cb_(1234);
      }
    });
    return true;
  }
  bool stop()
  {
    if (!t_.joinable()) return false;
    stop_ = true;
    t_.join();
    return true;
  }
  bool registerCallback(Callback cb)
  {
    if (t_.joinable()) return false;
    cb_ = cb;
    return true;
  }

private:
  std::thread t_;
  Callback cb_;
  std::atomic_bool stop_;
};

It works just fine and can be tested with this short example main.cpp:

#include <iostream>
#include "system.hpp"

int g_counter = 0;

void foo(int i)
{
  std::cout << i << std::endl;
  g_counter++;
}

int main()
{
  System s;
  s.registerCallback(foo);
  s.start();
  while (g_counter < 3)
  {
    std::this_thread::sleep_for(std::chrono::milliseconds(1));
  }
  s.stop();
  return 0;
}

which will output 1234 a few times and then will stop. However I encountered a problem trying to create python bindings for my System. If I register a python function as a callback, my program will deadlock after calling System::stop. I investigated the topic a bit and it seems that I face the issue with GIL. Reproducible example:

binding.cpp:

#include "pybind11/functional.h"
#include "pybind11/pybind11.h"

#include "system.hpp"

namespace py = pybind11;

PYBIND11_MODULE(mysystembinding, m) {
  py::class_<System>(m, "System")
    .def(py::init<>())
    .def("start", &System::start)
    .def("stop", &System::stop)
    .def("registerCallback", &System::registerCallback);
}

python script:

#!/usr/bin/env python

import mysystembinding
import time

g_counter = 0

def foo(i):
  global g_counter
  print(i)
  g_counter = g_counter + 1

s = mysystembinding.System()
s.registerCallback(foo)
s.start()
while g_counter < 3:
  time.sleep(1)
s.stop()

I have read the pybind11 docs section about the possibility to acquire or release GIL on the C++ side. However I did not manage to get rid of the deadlock that occurs in my case:

PYBIND11_MODULE(mysystembinding, m) {
  py::class_<System>(m, "System")
    .def(py::init<>())
    .def("start", &System::start)
    .def("stop", &System::stop)
    .def("registerCallback", [](System* s, System::Callback cb)
      {
        s->registerCallback([cb](int i)
        {
          // py::gil_scoped_acquire acquire;
          // py::gil_scoped_release release;
          cb(i);
        });
      });
}

If I call py::gil_scoped_acquire acquire; before calling the callback, deadlock occurs anyway. If I call py::gil_scoped_release release; before calling the callback, I get

Fatal Python error: PyEval_SaveThread: NULL tstate

What should I do to register python functions as callbacks and avoid deadlocks?

pptaszni
  • 5,591
  • 5
  • 27
  • 43

2 Answers2

5

Thanks to this discussion and many other resources (1, 2, 3) I figured out that guarding the functions that start and join the C++ thread with gil_scoped_release seems to solve the problem:

PYBIND11_MODULE(mysystembinding, m) {
  py::class_<System>(m, "System")
    .def(py::init<>())
    .def("start", &System::start, py::call_guard<py::gil_scoped_release>())
    .def("stop", &System::stop, py::call_guard<py::gil_scoped_release>())
    .def("registerCallback", &System::registerCallback);
}

Apparently deadlocks occurred because python was holding a lock while invoking a binding responsible for C++ thread manipulation. I am still not really sure if my reasoning is correct, so I would appreciate any expert's comments.

pptaszni
  • 5,591
  • 5
  • 27
  • 43
  • Don't you have to acquire the GIL in the callback, since it will have been released by the time you call it in the worker thread? Or is this done automatically by Pybind in the type conversion? – cade Sep 12 '20 at 17:52
  • I think you don't have to, if the callback is a python function. In [this example](https://pybind11.readthedocs.io/en/stable/advanced/misc.html) they acquire lock in case of C++ function calling some python code inside, here I have the opposite situation. But honestly I don't quite understand what exactly they are guarding in that example, since I don't see any parallel code there. – pptaszni Sep 13 '20 at 18:56
3

Call gil_scoped_release before join() will get rid of the deadlock in my case.

void Tick::WaitLifeOver() {
  if (thread_.joinable()) {
    thread_.join();
  }
}
PYBIND11_MODULE(tick_pb, m) {
  py::class_<Tick, std::shared_ptr<Tick>>(m, "Tick")
    // ...
    .def("wait_life_over", &Tick::WaitLifeOver,
        py::call_guard<py::gil_scoped_release>());
}

Here are the codes: C++ Thread Callback Python Function

kuokuo
  • 196
  • 1
  • 4
  • Thanks for your answer. However, please note that your solution is the same as the one I posted 6 months ago. Good to confirm it works for you as well, anyway. – pptaszni Aug 20 '20 at 06:55