Skip to content

Commit

Permalink
Fix and improvements to toward 3.13t (pytorch#136319)
Browse files Browse the repository at this point in the history
  • Loading branch information
albanD authored and pytorchmergebot committed Sep 20, 2024
1 parent e3ea542 commit cf31724
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 15 deletions.
2 changes: 1 addition & 1 deletion functorch/csrc/dim/dim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ struct Tensor : public mpy::base<Tensor> {

static mpy::obj<Tensor> create() {
if (!TensorType) {
TensorType = (PyTypeObject*) mpy::import("functorch.dim").attr("Tensor").ptr();
TensorType = (PyTypeObject*) mpy::import("functorch.dim").attr("Tensor").release();
}
return Tensor::alloc(TensorType);
}
Expand Down
4 changes: 3 additions & 1 deletion torch/csrc/Storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ static bool THPStorage_tryPreserve(THPStorage* self) {
TORCH_INTERNAL_ASSERT(!storage_impl->pyobj_slot()->owns_pyobj());

storage_impl->pyobj_slot()->set_owns_pyobj(true);
Py_INCREF(self);
// When resurrecting, we MUST use _Py_NewReference and not Py_INCREF to
// ensure the PyObject is in a valid state
_Py_NewReference((PyObject*)self);

self->cdata = c10::MaybeOwned<c10::Storage>::borrowed(storage);
return true;
Expand Down
26 changes: 13 additions & 13 deletions torch/csrc/autograd/python_variable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,19 +382,19 @@ static bool THPVariable_tryResurrect(THPVariable* self) {

tensor_impl->pyobj_slot()->set_owns_pyobj(true);

// Resurrect the Python object. This is something CPython does
// internally occasionally, see
// https://github.com/python/cpython/blob/b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985/Objects/object.c#L248-L259
// so we just copy the pattern here. Note that we don't have to worry
// about saving and restoring the refcount (as the quoted code does)
// because we actually DO need to reset the refcount to one here, we
// can't assume that some other code has taken care of it.
// NB: this will overreport _Py_RefTotal but based on inspection of object.c
// there is no way to avoid this
#ifdef Py_TRACE_REFS
_Py_AddToAllObjects(reinterpret_cast<PyObject*>(self), 1);
#endif
Py_INCREF(self);
// Resurrect the Python object. This is something CPython does
// internally occasionally, see
// https://github.com/python/cpython/blob/b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985/Objects/object.c#L248-L259
// so we just copy the pattern here. Note that we don't have to worry
// about saving and restoring the refcount (as the quoted code does)
// because we actually DO need to reset the refcount to one here, we
// can't assume that some other code has taken care of it.
// NB: this will overreport _Py_RefTotal but based on inspection of object.c
// there is no way to avoid this

// When resurrecting, we MUST use _Py_NewReference and not Py_INCREF to
// ensure the PyObject is in a valid state
_Py_NewReference((PyObject*)self);

// Flip THPVariable to be non-owning
// (near use-after-free miss here: fresh MaybeOwned is created breaking
Expand Down
12 changes: 12 additions & 0 deletions torch/csrc/jit/python/pybind_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ ToIValueAllowNumbersAsTensors::~ToIValueAllowNumbersAsTensors() {
// C++->Python. We need this because otherwise we may get the old Python object
// if C++ creates a new object at the memory location of the deleted object.
void clear_registered_instances(void* ptr) {
#if IS_PYBIND_2_13_PLUS
py::detail::with_instance_map(
ptr, [&](py::detail::instance_map& registered_instances) {
auto range = registered_instances.equal_range(ptr);
for (auto it = range.first; it != range.second; ++it) {
auto vh = it->second->get_value_and_holder();
vh.set_instance_registered(false);
}
registered_instances.erase(ptr);
});
#else
auto& registered_instances =
pybind11::detail::get_internals().registered_instances;
auto range = registered_instances.equal_range(ptr);
Expand All @@ -41,6 +52,7 @@ void clear_registered_instances(void* ptr) {
vh.set_instance_registered(false);
}
registered_instances.erase(ptr);
#endif
}

// WARNING: Precondition for this function is that, e.g., you have tested if a
Expand Down
2 changes: 2 additions & 0 deletions torch/csrc/utils/pybind.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

namespace py = pybind11;

#define IS_PYBIND_2_13_PLUS PYBIND11_VERSION_HEX >= 0x020D0000

// This makes intrusive_ptr to be available as a custom pybind11 holder type,
// see
// https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html#custom-smart-pointers
Expand Down

0 comments on commit cf31724

Please sign in to comment.