Python 2: Remove assert that failed (false positive) when raising an old-style instance.

2 files changed, 20 insertions, 6 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 3e1fd8c..2576ee5 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,13 +5,14 @@
 2.0.0rc3 (unreleased)
 =====================
 
-- Nothing changed yet.
+- Python 2: Fix a crash that could occur when raising an old-style
+  instance object.
 
 
 2.0.0rc2 (2022-10-28)
 =====================
 
-- Workaround `CPython 3.8 bug
+- Workaround `a CPython 3.8 bug
   <https://github.com/python/cpython/issues/81308>`_ that could cause
   the interpreter to crash during an early phase of shutdown with the
   message "Fatal Python error: Python memory allocator called without
diff --git a/src/greenlet/greenlet_refs.hpp b/src/greenlet/greenlet_refs.hpp
index 5551397..622880b 100644
--- a/src/greenlet/greenlet_refs.hpp
+++ b/src/greenlet/greenlet_refs.hpp
@@ -1002,7 +1002,10 @@ namespace greenlet {
 
             }
             else if (PyExceptionInstance_Check(type)) {
-                /* Raising an instance. The value should be a dummy. */
+                /* Raising an instance --- usually that means an
+                   object that is a subclass of BaseException, but on
+                   Python 2, that can also mean an arbitrary old-style
+                   object. The value should be a dummy. */
                 if (instance && !instance.is_None()) {
                     throw PyErrOccurred(
                                     PyExc_TypeError,
@@ -1010,11 +1013,21 @@ namespace greenlet {
                 }
                 /* Normalize to raise <class>, <instance> */
                 this->instance = this->type;
-#ifndef NDEBUG
+                this->type = PyExceptionInstance_Class(instance.borrow());
+
+                /*
+                  It would be tempting to do this:
+
                 Py_ssize_t type_count = Py_REFCNT(Py_TYPE(instance.borrow()));
-#endif
                 this->type = PyExceptionInstance_Class(instance.borrow());
-                assert(type.REFCNT() == type_count + 1);
+                assert(this->type.REFCNT() == type_count + 1);
+
+                But that doesn't work on Python 2 in the case of
+                old-style instances: The result of Py_TYPE is going to
+                be the global shared <type instance> that all
+                old-style classes have, while the return of Instance_Class()
+                will be the Python-level class object. The two are unrelated.
+                */
             }
             else {
                 /* Not something you can raise. throw() fails. */