wlav / cppyy

Other
400 stars 41 forks source link

Calling C++ from multiple Python threads breaks sometimes #61

Closed levy closed 2 years ago

levy commented 2 years ago

I want to start with saying that this tool is so nice that it feels like it's cheating.

Anyway, my guess for this one is that there's a threading issue while the wrapper code is being genarated?!

pip3 list:

cppyy                   2.3.1
cppyy-backend           1.14.8
cppyy-cling             6.25.3
CPyCppyy                1.12.10

test.h:

#include <string>
#include <map>

class Simulation
{
  public:
    virtual void set_something(std::map<std::string, std::string>, std::string);
};

test.cc:

#include "test.h"

void Simulation::set_something(std::map<std::string, std::string>, std::string)
{
}

test.py:

import cppyy
import threading

cppyy.add_include_path(".")
cppyy.add_library_path(".")
cppyy.load_library("libtest")
cppyy.include("test.h")

def test():
    o = {"a": "b"}
    simulation = cppyy.gbl.Simulation()
    simulation.set_something(o, ".")

for i in range(0, 100):
    threading.Thread(target=test).start()

compile:

clang++ -fPIC -shared -o libtest.so test.cc

run:

evy@valardohaeris:~$ python3 test.py
Exception in thread Thread-2 (test):
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 946, in run
    self._target(*self._args, **self._kwargs)
  File "/home/levy/test.py", line 12, in test
    simulation.set_something(o, ".")
TypeError: void Simulation::set_something(std::map<std::string,std::string>, std::string) =>
    TypeError: takes at most 2 arguments (3 given)

Please note that the function takes 2 arguments and it is called with 2 arguments. This error doesn't always come up, only if multiple threads are used, and even then it only pops up occasionally.

wlav commented 2 years ago

Not sure yet where exactly the problem is, but a workaround is to not rely on implicit conversion of dict -> std::map:

o = cppyy.gbl.std.map({"a": "b"})
levy commented 2 years ago

Indeed, this workaround fixes the problem. I also have Python list to std::vector implicit conversion, but once I make them explicit they work too.

wlav commented 2 years ago

Same problem, same fix as #62.

wlav commented 2 years ago

Aside, the reason why the problem cropped up more with the implicit conversion, is because that is done through Python API calls: both to get the data out of the dict and into the std::map. With these extra calls, there's a higher chance that threads will be switched while still mid-call. Otherwise, the C++ portion of the bindings is just 1 "Python instruction" and threads don't switch during it.

wlav commented 2 years ago

Released with 2.4.0 and its dependencies.