Fix a manylinux crasher.

We were using self->python_state.tp_clear() when we meant to use this->python_state.tp_clear().

The extra self had already been cleared, so it was calling into a nullptr.

In member methods, always use this.

4 files changed, 43 insertions, 9 deletions

diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp
index 5721310..c4da3e7 100644
--- a/src/greenlet/greenlet.cpp
+++ b/src/greenlet/greenlet.cpp
@@ -980,7 +980,7 @@ Greenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) G_NO
     /* stack variables from above are no good and also will not unwind! */
     // EXCEPT: That can't be true, we access run, among others, here.
 
-    self->stack_state.set_active(); /* running */
+    this->stack_state.set_active(); /* running */
 
     // XXX: We could clear this much earlier, right?
     // Or would that introduce the possibility of running Python
@@ -1350,6 +1350,7 @@ green_new(PyTypeObject* type, PyObject* UNUSED(args), PyObject* UNUSED(kwds))
         (PyGreenlet*)PyBaseObject_Type.tp_new(type, mod_globs.empty_tuple, mod_globs.empty_dict);
     if (o) {
         new Greenlet(o, GET_THREAD_STATE().state().borrow_current());
+        //cerr << "For PyGreenlet at " << o << " allocted Greenlet at " << p << endl;
         assert(Py_REFCNT(o) == 1);
     }
     return o;
@@ -1457,7 +1458,7 @@ Greenlet::deallocing_greenlet_in_thread(const ThreadState& thread_state)
         // exception happened. Whether or not an exception happens,
         // we need to restore the parent in case the greenlet gets
         // resurrected.
-        self->throw_GreenletExit();
+        this->throw_GreenletExit();
         return;
     }
 
@@ -1533,6 +1534,13 @@ green_traverse(PyGreenlet* self, visitproc visit, void* arg)
     //    greenlet, solves several leaks for us.
 
     Py_VISIT(self->dict);
+    if (!self->pimpl) {
+        cerr << "ERROR: For PyGreenlet at " << self
+             << " Traversing into deleted Greenlet!"
+             << endl;
+        return 0;
+    }
+
     return self->pimpl->tp_traverse(visit, arg);
 }
 
@@ -1576,8 +1584,8 @@ Greenlet::tp_clear()
     this->_run_callable.CLEAR();
 
 
-    self->python_state.tp_clear(own_top_frame);
-    self->exception_state.tp_clear();
+    this->python_state.tp_clear(own_top_frame);
+    this->exception_state.tp_clear();
     return 0;
 }
 
@@ -1693,8 +1701,14 @@ green_dealloc(PyGreenlet* self)
     Py_CLEAR(self->dict);
 
     if (self->pimpl) {
-        delete self->pimpl;
+        //cerr << "For PyGreenlet at " << self << " deallocating
+        //Greenlet at " << self->pimpl << endl;
+        // In case deleting this, which frees some memory,
+        // somewhow winds up calling back into us. That's usually a
+        //bug in our code.
+        Greenlet* p = self->pimpl;
         self->pimpl = nullptr;
+        delete p;
     }
     // and finally we're done. self is now invalid.
     Py_TYPE(self)->tp_free((PyObject*)self);
diff --git a/src/greenlet/greenlet_allocator.hpp b/src/greenlet/greenlet_allocator.hpp
index a182e0e..adad67a 100644
--- a/src/greenlet/greenlet_allocator.hpp
+++ b/src/greenlet/greenlet_allocator.hpp
@@ -4,7 +4,7 @@
 #include <Python.h>
 #include <memory>
 #include "greenlet_compiler_compat.hpp"
-
+// #include <iostream>
 
 namespace greenlet
 {
@@ -33,6 +33,13 @@ namespace greenlet
 
         T* allocate(size_t number_objects, const void* UNUSED(hint)=0)
         {
+            // using std::cerr;
+            // using std::endl;
+
+            // cerr << "Allocating " << number_objects
+            //      << " at " << UNUSED_hint
+            //      << " from " << typeid(this).name()
+            //      << endl;
             void* p;
             if (number_objects == 1)
                 p = PyObject_Malloc(sizeof(T));
@@ -43,6 +50,13 @@ namespace greenlet
 
         void deallocate(T* t, size_t n)
         {
+            // using std::cerr;
+            // using std::endl;
+            // cerr << "Deallocating " << n
+            //      << " at " << t
+            //      << " of " << typeid(t).name()
+            //      << " from " << typeid(this).name()
+            //      << endl;
             void* p = t;
             if (n == 1) {
                 PyObject_Free(p);
diff --git a/src/greenlet/greenlet_greenlet.hpp b/src/greenlet/greenlet_greenlet.hpp
index 1bdd375..01c5a51 100644
--- a/src/greenlet/greenlet_greenlet.hpp
+++ b/src/greenlet/greenlet_greenlet.hpp
@@ -67,6 +67,11 @@ namespace greenlet
             return this->_context;
         }
 
+        inline void tp_clear()
+        {
+            this->_context.CLEAR();
+        }
+
         template<typename T>
         inline static PyObject* context(T* tstate)
         {
@@ -96,6 +101,8 @@ namespace greenlet
             throw AttributeError("no context");
         }
 
+        inline void tp_clear(){};
+
         template<typename T>
         inline static PyObject* context(T* UNUSED(tstate))
         {
@@ -732,9 +739,7 @@ int PythonState::tp_traverse(visitproc visit, void* arg, bool own_top_frame) G_N
 
 void PythonState::tp_clear(bool own_top_frame) G_NOEXCEPT
 {
-#if GREENLET_PY37
-    this->_context.CLEAR();
-#endif
+    PythonStateContext::tp_clear();
     // If we get here owning a frame,
     // we got dealloc'd without being finished. We may or may not be
     // in the same thread.
diff --git a/src/greenlet/greenlet_refs.hpp b/src/greenlet/greenlet_refs.hpp
index 1061a6a..c7935b2 100644
--- a/src/greenlet/greenlet_refs.hpp
+++ b/src/greenlet/greenlet_refs.hpp
@@ -413,6 +413,7 @@ namespace greenlet {
         void CLEAR()
         {
             Py_CLEAR(this->p);
+            assert(this->p == nullptr);
         }
     };