8000 Fix and improvements to toward 3.13t (#136319) · albanD/pytorch@d5f26b5 · GitHub
[go: up one dir, main page]
Skip to content

Commit d5f26b5

Browse files
albanDalbanD
committed
Fix and improvements to toward 3.13t (pytorch#136319)
Small part of pytorch#130689 Pull Request resolved: pytorch#136319 Approved by: https://github.com/malfet, https://github.com/Skylion007
1 parent 0c2b29e commit d5f26b5

File tree

5 files changed

+31
-15
lines changed

5 files changed

+31
-15
lines changed

‎functorch/csrc/dim/dim.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ struct Tensor : public mpy::base<Tensor> {
739739

740740
static mpy::obj<Tensor> create() {
741741
if (!TensorType) {
742-
TensorType = (PyTypeObject*) mpy::import("functorch.dim").attr("Tensor").ptr();
742+
TensorType = (PyTypeObject*) mpy::import("functorch.dim").attr("Tensor").release();
743743
}
744744
return Tensor::alloc(TensorType);
745745
}

‎torch/csrc/Storage.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,9 @@ static bool THPStorage_tryPreserve(THPStorage* self) {
195195
TORCH_INTERNAL_ASSERT(!storage_impl->pyobj_slot()->owns_pyobj());
196196

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

200202
self->cdata = c10::MaybeOwned<c10::Storage>::borrowed(storage);
201203
return true;

‎torch/csrc/autograd/python_variable.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -382,19 +382,19 @@ static bool THPVariable_tryResurrect(THPVariable* self) {
382382

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

385-
// Resurrect the Python object. This is something CPython does
386-
// internally occasionally, see
387-
// https://github.com/python/cpython/blob/b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985/Objects/object.c#L248-L259
388-
// so we just copy the pattern here. Note that we don't have to worry
389-
// about saving and restoring the refcount (as the quoted code does)
390-
// because we actually DO need to reset the refcount to one here, we
391-
// can't assume that some other code has taken care of it.
392-
// NB: this will overreport _Py_RefTotal but based on inspection of object.c
393-
// there is no way to avoid this
394-
#ifdef Py_TRACE_REFS
395-
_Py_AddToAllObjects(reinterpret_cast<PyObject*>(self), 1);
396-
#endif
397-
Py_INCREF(self);
385+
// Resurrect the Python object. This is something CPython does
386+
// internally occasionally, see
387+
// https://github.com/python/cpython/blob/b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985/Objects/object.c#L248-L259
388+
// so we just copy the pattern here. Note that we don't have to worry
389+
// about saving and restoring the refcount (as the quoted code does)
390+
// because we actually DO need to reset the refcount to one here, we
391+
// can't assume that some other code has taken care of it.
392+
// NB: this will overreport _Py_RefTotal but based on inspection of object.c
393+
// there is no way to avoid this
394+
395+
// When resurrecting, we MUST use _Py_NewReference and not Py_INCREF to
396+
// ensure the PyObject is in a valid state
397+
_Py_NewReference((PyObject*)self);
398398

399399
// Flip THPVariable to be non-owning
400400
// (near use-after-free miss here: fresh MaybeOwned is created breaking

‎torch/csrc/jit/python/pybind_utils.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ ToIValueAllowNumbersAsTensors::~ToIValueAllowNumbersAsTensors() {
3333
// C++->Python. We need this because otherwise we may get the old Python object
3434
// if C++ creates a new object at the memory location of the deleted object.
3535
void clear_registered_instances(void* ptr) {
36+
#if IS_PYBIND_2_13_PLUS
37+
py::detail::with_instance_map(
38+
ptr, [&](py::detail::instance_map& registered_instances) {
39+
auto range = registered_instances.equal_range(ptr);
40+
for (auto it = range.first; it != range.second; ++it) {
41+
auto vh = it->second->get_value_and_holder();
42+
vh.set_instance_registered(false);
43+
}
44+
registered_instances.erase(ptr);
45+
});
46+
#else
3647
auto& registered_instances =
3748
pybind11::detail::get_internals().registered_instances;
3849
auto range = registered_instances.equal_range(ptr);
@@ -41,6 +52,7 @@ void clear_registered_instances(void* ptr) {
4152
vh.set_instance_registered(false);
4253
}
4354
registered_instances.erase(ptr);
55+
#endif
4456
}
4557

4658
// WARNING: Precondition for this function is that, e.g., you have tested if a

‎torch/csrc/utils/pybind.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
namespace py = pybind11;
2121

22+
#define IS_PYBIND_2_13_PLUS PYBIND11_VERSION_HEX >= 0x020D0000
23+
2224
// This makes intrusive_ptr to be available as a custom pybind11 holder type,
2325
// see
2426
// https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html#custom-smart-pointers

0 commit comments

Comments
 (0)
0