pybind / pybind/pybind11

[BUG]: Silent failure if python object is destroyed but shared_ptr to CPP object is held

Open
#4,603 4 comments 0 reactions 1 assignee View on GitHub

@rwgk is already working on this.

Since Apr 25, 2023.

triage
Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Required prerequisites
What version (or hash if on master) of pybind11 are you using?

2.10.4

Problem description

We have come across an issue concerning the combination of Python and CPP object lifetimes. If a Cpp class is exposed via pybind11 using a shared_ptr as a holder, it is possible to destroy the python object while retaining a shared_ptr to the Cpp object. However, the Cpp object behaviour reverts to the Cpp base class and all python behaviour is silently dropped.

Is the silent drop expected? We would either expect an exception when the object is used or to retain full functionality. We attached some simple example code below:

Reproducible example code

On the Cpp side:

#include <gtest/gtest.h>
#include <pybind11/embed.h>
namespace py = pybind11;

#include <memory>

class Parent {
 public:
  Parent() = default;
  virtual ~Parent() = default;

  virtual int Return() const { return 1; }
};
using ParentPtr = std::shared_ptr<Parent>;

class Child : public Parent {
 public:
  Child() = default;
  virtual ~Child() = default;

  int Return() const override { return 2; }
};
using ChildPtr = std::shared_ptr<Child>;

template <typename Base = Parent>
class PyTrampoline : public Base {
 public:
  using Base::Base;
  virtual ~PyTrampoline() = default;

  int Return() const override { PYBIND11_OVERLOAD(int, Base, Return, ); }
};

PYBIND11_EMBEDDED_MODULE(duality, module) {
  py::class_<Parent, PyTrampoline<>, ParentPtr>(module, "Parent")  //
      .def(py::init<>())                                           //
      .def("Return", &Parent::Return);
  py::class_<Child, Parent, PyTrampoline<Child>, ChildPtr>(module, "Child")  //
      .def(py::init<>());
}

void checkPythonResult(const std::string& pClassName, int pPythonExpected,
                       int pCppExpected) {
  auto dualityModule = py::module_::import("Duality");
  auto python = dualityModule.attr(pClassName.c_str())();

  auto result = python.attr("Return")();
  EXPECT_EQ(result.cast<int>(), pPythonExpected);

  auto parentPtr = python.cast<ParentPtr>();
  ASSERT_NE(parentPtr, nullptr);
  EXPECT_EQ(parentPtr->Return(), pPythonExpected);

  ChildPtr childPtr;
  try {
    childPtr = python.cast<ChildPtr>();
    ASSERT_NE(childPtr, nullptr);
    EXPECT_EQ(childPtr->Return(), pPythonExpected);
  } catch (std::exception&) {
  }

  if (childPtr != nullptr) EXPECT_EQ(childPtr->Return(), pPythonExpected);

  python = py::none();
  EXPECT_EQ(parentPtr->Return(), pCppExpected);
  if (childPtr != nullptr) EXPECT_EQ(childPtr->Return(), pCppExpected);
}

TEST(Utilities, Duality) {
  EXPECT_EQ(std::make_shared<Parent>()->Return(), 1);
  EXPECT_EQ(std::make_shared<Child>()->Return(), 2);

  py::initialize_interpreter(false);

  checkPythonResult("Parent", 1, 1);
  checkPythonResult("Child", 2, 2);
  checkPythonResult("PythonParent", 3, 1);
  checkPythonResult("PythonChild", 4, 2);

  py::finalize_interpreter();
}

On the Python side

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

from duality import Parent, Child


class PythonParent(Parent):
    def __init__(self) -> None:
        Parent.__init__(self)

    def Return(self) -> int:
        return 3


class PythonChild(Child):
    def __init__(self) -> None:
        Child.__init__(self)

    def Return(self) -> int:
        return 4

And finally a CMakeLists.txt file to build

cmake_minimum_required(VERSION 3.4)
project(Duality)

set(CMAKE_CXX_STANDARD 14)

include(FetchContent)
FetchContent_Declare(googletest GIT_REPOSITORY "https://github.com/google/googletest" GIT_TAG "v1.13.0" GIT_SHALLOW ON TEST_BEFORE_INSTALL ON)
FetchContent_MakeAvailable(googletest)

find_package(pybind11 REQUIRED)

add_executable(DualityTest DualityTest.cpp)
target_link_libraries(DualityTest PRIVATE pybind11::embed)
target_link_libraries(DualityTest PRIVATE GTest::gtest_main)
Is this a regression? Put the last known working version here if it is.

Not a regression

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.