Skip to content
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

Child Class Method Not Being Called as Expected, Base Class Method Called Instead #1552

Closed
bgoodman44 opened this issue Oct 4, 2018 · 9 comments
Assignees
Labels
holders Issues and PRs regarding holders

Comments

@bgoodman44
Copy link

Hello,

I have a simple test case for polymorphism that isn't working quite right.. I have two C++ classes, Client and Dispatcher. Dispatcher has a method called "Dispatch" that takes a Client* as an input, and calls the the clients virtual ProcessEvent() method. I've made both of these classes available to python using the code below. I've derived from the Client class as the python class SomeClient, and overridden its ProcessEvent function. When I call "Dispatch" from anywhere EXCEPT inside the SomeClient::ProcessEvent method, everything works as expected, i get a report from the child class. But when I call Dispatch from within SomeClient::ProcessEvent, the base classes ProcessEvent is called. Any ideas what could be going wrong here? I expect to get an infinite recursion that results in SomeClient::ProcessEvent being called forever.

C++:

#include <iostream>
#include "pybind11/pybind11.h"

namespace py = pybind11;
class Dispatcher;
class Client
{
public:
    Client(Dispatcher* disp): PtrD(disp)
    {
        std::cout << "In Client::Client\n";
    }
    virtual ~Client(){};
    virtual void ProcessEvent()
    {
        std::cout << "THIS SHOULDN'T HAPPEN --In Client::ProcessEvent\n";
    }
    Dispatcher* PtrD;
};
class Dispatcher
{
public:
    Dispatcher()
    {
        std::cout << "In Dispatcher::Dispatcher\n";
    }
    virtual ~Dispatcher(){};

    void Dispatch(Client* client)
    {
        std::cout << "Dispatcher::Dispatch called by " << client << std::endl;
        client->ProcessEvent();
    }
};
class DispatcherTrampoline : public Dispatcher
{
public:
    using Dispatcher::Dispatcher;
};
class ClientTrampoline : public Client
{
public:
    using Client::Client;

    void ProcessEvent() override
    {
        PYBIND11_OVERLOAD(void,Client,ProcessEvent,);
    }
};
PYBIND11_MODULE(libTestCase,mod)
{
    py::class_<Client,ClientTrampoline> cli(mod,"Client");
    cli.def(py::init<Dispatcher* >());
    cli.def("ProcessEvent",&Client::ProcessEvent);
    cli.def_readwrite("PtrD",&Client::PtrD);

    py::class_<Dispatcher,DispatcherTrampoline> dsp(mod,"Dispatcher");
    dsp.def(py::init< >());
    dsp.def("Dispatch",&Dispatcher::Dispatch);
}

Python Test:

from build.libTestCase import Client,Dispatcher

class SomeClient(Client):
    def __init__(self,d):
        print("In SomeClient::__init__")
        super().__init__(d);

    def ProcessEvent(self):
        print("in SomeClient::ProcessEvent,about to call self.ProcessEvent")
        self.PtrD.Dispatch(self);

if __name__ == "__main__":
    dd = Dispatcher()
    cl = SomeClient(dd)
    dd.Dispatch(cl)

TERMINAL OUTPUT:

In Dispatcher::Dispatcher
In SomeClient::__init__
In Client::Client
Dispatcher::Dispatch called by 0x20bb270
in SomeClient::ProcessEvent,about to call self.ProcessEvent
Dispatcher::Dispatch called by 0x20bb270
THIS SHOULDN'T HAPPEN --In Client::ProcessEvent
@bgoodman44
Copy link
Author

I'll pay actual money for a solution to my problem!!

https://www.upwork.com/job/Fix-bug-Simple-Python-PyBind11-Module_~01155fc34bd0078416/

@bgoodman44
Copy link
Author

bgoodman44 commented Oct 8, 2018

@bgoodman44
Copy link
Author

Uploaded project sources and CMakeList file. Also included a version written in pure python to show how it should work.

Files.zip

@vokama
Copy link

vokama commented Oct 8, 2018

After I've seen your job proposition was closed, I've decided to at least create an issue based on my experience with this problem, which led me to this page.

I am fairly certain it's a pybind11 bug, but its nature is a mystery. At first I thought it would be similiar to #1389, but SomeClient object is alive for the entire duration of a program.

It seems that the problem is caused by the following sequence:
pythonFuncA->cppFuncA->pythonFuncA.
Now if cppFuncA is called by any function except pythonFuncA, then everything works as expected. Thus adding a wrapper for cpp function call is an actual workaround:
pythonFuncA->pythonFuncB->cppFuncA->pythonFuncA

@cmdawson
Copy link

cmdawson commented Oct 9, 2018

In get_overload_type there is a check on the current PyFrameObject to ensure it's not resolving an overload from within a function of the same name.

(std::string) str(frame->f_code->co_name) == name

(both sides of this are ProcessEvent in the second call). It looks like this precludes any recursive calls to a virtual function overloaded in python.

@EricCousineau-TRI
Copy link
Collaborator

EricCousineau-TRI commented Dec 29, 2020

Possibly relates:

@bgoodman44 Even though it's 2 years late, do you still have stake in this issue? If so, can you see #2092 might fix your issue?

@EricCousineau-TRI EricCousineau-TRI self-assigned this Dec 29, 2020
@EricCousineau-TRI
Copy link
Collaborator

EricCousineau-TRI commented Jan 7, 2021

FYI Rather than #2092, you can try the later PR, #2772

EDIT: Or rather, can you try latest master? @YannickJadoul's PR, #2564?
I also have details on why that supersedes my PR: #2772 (comment)

@EricCousineau-TRI
Copy link
Collaborator

Trying out repro - can reproduce your issue on latest master:
master...EricCousineau-TRI:issue-1552-repro

@EricCousineau-TRI
Copy link
Collaborator

I think it was because you relied on an odd recursive dispatch between Client.ProcessEvent() and Dispatcher.Dispatch(). (That doesn't appear to be double-dispatch, b/c you're not really resolving to concrete overloads on the first virtual call)

I simplified your example, and could make it work: 597752a

$ cd pybind11
$ mkdir build && cd build && cmake .. && env PYTEST_ADDOPTS="-s -x" make pytest
../../tests/test_issue1552.py In Dispatcher::Dispatcher
...
In SomeClient::__init__
In Client::Client
Dispatcher::Dispatch called by 0x1491850
Python ProcessEvent

For more details on how to configure the tests, please see:
https://github.com/pybind/pybind11/blob/08bca374fdcf3831e8ad15246fbd3b2d5675b4a5/.github/CONTRIBUTING.md

Closing for now.

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

No branches or pull requests

4 participants