MNT: explicitly borrow the PyObject pointer for wrappers

tacaswell · tacaswell · commit 50f878614de1 · 2022-04-30T16:49:20.000-04:00
When calling the CPython C api, always explicitly borrow the PyObject pointer.
diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp
@@ -2392,7 +2392,7 @@ green_repr(BorrowedGreenlet self)
     PyObject* result;
     int never_started = !self->started() && !self->active();
 
-    const char* const tp_name = Py_TYPE(self)->tp_name;
+    const char* const tp_name = Py_TYPE(self.borrow())->tp_name;
 
     if (_green_not_dead(self)) {
         /* XXX: The otid= is almost useless because you can't correlate it to
diff --git a/src/greenlet/greenlet_refs.hpp b/src/greenlet/greenlet_refs.hpp
@@ -990,7 +990,7 @@ namespace greenlet {
                                     "throw() third argument must be a traceback object");
             }
 
-            if (PyExceptionClass_Check(type)) {
+            if (PyExceptionClass_Check(type.borrow())) {
                 // If we just had a type, we'll now have a type and
                 // instance.
                 // The type's refcount will have gone up by one
@@ -1000,7 +1000,7 @@ namespace greenlet {
                 PyErr_NormalizeException(&type, &instance, &traceback);
 
             }
-            else if (PyExceptionInstance_Check(type)) {
+            else if (PyExceptionInstance_Check(type.borrow())) {
                 /* Raising an instance. The value should be a dummy. */
                 if (instance && !instance.is_None()) {
                     throw PyErrOccurred(
diff --git a/src/greenlet/greenlet_thread_state.hpp b/src/greenlet/greenlet_thread_state.hpp
@@ -397,8 +397,8 @@ class ThreadState {
                     }
                     else if (refs
                              && refs.size() == 1
-                             && PyCFunction_Check(refs.at(0))
-                             && Py_REFCNT(refs.at(0)) == 2) {
+                             && PyCFunction_Check(refs.at(0).borrow())
+                             && Py_REFCNT(refs.at(0).borrow()) == 2) {
                         assert(refs.REFCNT() == 1);
                         // Ok, we found a C method that refers to the
                         // main greenlet, and its only referenced
@@ -419,7 +419,8 @@ class ThreadState {
                             if (refs && refs.empty()) {
                                 // Nope, it can't be found so it won't
                                 // ever be GC'd. Drop it.
-                                Py_CLEAR(function_w);
+                                PyObject * tmp = function_w.borrow();
+                                Py_CLEAR(tmp);
                             }
                         }
                     }

Original file line number	Diff line number	Diff line change
`@@ -397,8 +397,8 @@ class ThreadState {`
`397`	`397`	`}`
`398`	`398`	`else if (refs`
`399`	`399`	`&& refs.size() == 1`
`400`		`- && PyCFunction_Check(refs.at(0))`
`401`		`- && Py_REFCNT(refs.at(0)) == 2) {`
	`400`	`+ && PyCFunction_Check(refs.at(0).borrow())`
	`401`	`+ && Py_REFCNT(refs.at(0).borrow()) == 2) {`
`402`	`402`	`assert(refs.REFCNT() == 1);`
`403`	`403`	`// Ok, we found a C method that refers to the`
`404`	`404`	`// main greenlet, and its only referenced`
`@@ -419,7 +419,8 @@ class ThreadState {`
`419`	`419`	`if (refs && refs.empty()) {`
`420`	`420`	`// Nope, it can't be found so it won't`
`421`	`421`	`// ever be GC'd. Drop it.`
`422`		`- Py_CLEAR(function_w);`
	`422`	`+ PyObject * tmp = function_w.borrow();`
	`423`	`+ Py_CLEAR(tmp);`
`423`	`424`	`}`
`424`	`425`	`}`
`425`	`426`	`}`