Skip to content

Fix async Python functors invoking from multiple C++ threads (#1587) #1595

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

Merged
merged 2 commits into from
Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions include/pybind11/functional.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,20 @@ struct type_caster<std::function<Return(Args...)>> {
}
}

value = [func](Args... args) -> Return {
// ensure GIL is held during functor destruction
struct func_handle {
function f;
func_handle(function&& f_) : f(std::move(f_)) {}
func_handle(const func_handle&) = default;
~func_handle() {
gil_scoped_acquire acq;
function kill_f(std::move(f));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more clear to explicitly reset the function handle and decrease the reference count: f.release().dec_ref();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Done.

Copy link
Contributor Author

@uentity uentity Dec 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidhewitt causes strange test failure on VS with Python 3.6 on x86 platform, so reverted back to original version.
Logs: https://ci.appveyor.com/project/wjakob/pybind11/builds/20894165

}
};

value = [hfunc = func_handle(std::move(func))](Args... args) -> Return {
gil_scoped_acquire acq;
object retval(func(std::forward<Args>(args)...));
object retval(hfunc.f(std::forward<Args>(args)...));
/* Visual studio 2015 parser issue: need parentheses around this expression */
return (retval.template cast<Return>());
};
Expand Down
19 changes: 19 additions & 0 deletions tests/test_callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "pybind11_tests.h"
#include "constructor_stats.h"
#include <pybind11/functional.h>
#include <thread>


int dummy_function(int i) { return i + 1; }
Expand Down Expand Up @@ -146,4 +147,22 @@ TEST_SUBMODULE(callbacks, m) {
py::class_<CppBoundMethodTest>(m, "CppBoundMethodTest")
.def(py::init<>())
.def("triple", [](CppBoundMethodTest &, int val) { return 3 * val; });

// test async Python callbacks
using callback_f = std::function<void(int)>;
m.def("test_async_callback", [](callback_f f, py::list work) {
// make detached thread that calls `f` with piece of work after a little delay
auto start_f = [f](int j) {
auto invoke_f = [f, j] {
std::this_thread::sleep_for(std::chrono::milliseconds(50));
f(j);
};
auto t = std::thread(std::move(invoke_f));
t.detach();
};

// spawn worker threads
for (auto i : work)
start_f(py::cast<int>(i));
});
}
29 changes: 29 additions & 0 deletions tests/test_callbacks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest
from pybind11_tests import callbacks as m
from threading import Thread


def test_callbacks():
Expand Down Expand Up @@ -105,3 +106,31 @@ def test_function_signatures(doc):

def test_movable_object():
assert m.callback_with_movable(lambda _: None) is True


def test_async_callbacks():
# serves as state for async callback
class Item:
def __init__(self, value):
self.value = value

res = []

# generate stateful lambda that will store result in `res`
def gen_f():
s = Item(3)
return lambda j: res.append(s.value + j)

# do some work async
work = [1, 2, 3, 4]
m.test_async_callback(gen_f(), work)
# wait until work is done
from time import sleep
sleep(0.5)
assert sum(res) == sum([x + 3 for x in work])


def test_async_async_callbacks():
t = Thread(target=test_async_callbacks)
t.start()
t.join()