Merge pull request #429 from python-greenlet/issue419redux

Issue419redux

.github/workflows/tests.yml

@@ -58,19 +58,26 @@ jobs:
     - name: Install greenlet (non-Mac)
       if: ${{ ! startsWith(runner.os, 'Mac') }}
       run: |
-        python -m pip wheel --wheel-dir ./dist .
+        # Stupid setuptools doesn't want you running 'python setup.py' anymore,
+        # but stupid pip hides all the intersting compiler output by default, and the
+        # only way to get anything useful out is to ask *everything* to be verbose,
+        # which is much more junk than we need to wade through, making it hard to
+        # see what we want. What's the point of having warnings at all if we can't
+        # see them, though?
+        python -m pip wheel -v --wheel-dir ./dist .
         python -m pip install -U -e ".[test,docs]"
       env:
         # Ensure we test with assertions enabled.
         # As opposed to the manylinux builds, which we distribute and
         # thus only use O3 (because Ofast enables fast-math, which has
         # process-wide effects), we test with Ofast here, because we
         # expect that some people will compile it themselves with that setting.
-        CPPFLAGS: "-Ofast -UNDEBUG"
+        CPPFLAGS: "-Ofast -UNDEBUG -Wall"
+        CFLAGS:   "-Ofast -UNDEBUG -Wall"
     - name: Install greenlet (Mac)
       if: startsWith(runner.os, 'Mac')
       run: |
-        python -m pip wheel --wheel-dir ./dist .
+        python -m pip wheel -v --wheel-dir ./dist .
         python -m pip install -U -e ".[test,docs]"
         ls -l dist
         # Something in the build system isn't detecting that we're building for both,
@@ -84,7 +91,8 @@ jobs:
       env:
         # Unlike the above, we are actually distributing these
         # wheels, so they need to be built for production use.
-        CPPFLAGS: "-O3"
+        CPPFLAGS: "-O3 -flto -ffunction-sections"
+        CFLAGS:   "-O3 -flto -ffunction-sections"
         # Build for both architectures
         ARCHFLAGS: "-arch x86_64 -arch arm64"
 

CHANGES.rst

@@ -5,7 +5,8 @@
 3.1.1 (unreleased)
 ==================
 
-- Fix crashes on 32-bit PPC Linux.
+- Fix crashes on 32-bit PPC Linux. Note that there is no CI for this,
+  and support is best effort; there may be other issues lurking.
   See `issue 422
   <https://github.com/python-greenlet/greenlet/issues/422>`_.
 - Remove unnecessary logging sometimes during interpreter shutdown.

make-manylinux

@@ -22,7 +22,7 @@ if [ -d /greenlet -a -d /opt/python ]; then
     # Note: -Ofast includes -ffast-math which affects process-wide floating-point flags (e.g. can affect numpy).
     # It may also violate standards compliance in a few ways. Rather than opt-out with -fno-fast-math,
     # we use O3, which has neither of those problems.
-    export CFLAGS="-O3 -DNDEBUG"
+    export CFLAGS="-O3 -DNDEBUG -Wall"
     # Build in an isolated directory
     mkdir /tmp/build
     cd /tmp/build
@@ -45,7 +45,7 @@ if [ -d /greenlet -a -d /opt/python ]; then
 
         python -mpip install -U pip
         python -mpip install -U setuptools wheel
-        python -mpip wheel --wheel-dir ./dist .
+        python -mpip wheel -v --wheel-dir ./dist .
         python -mpip install -U .[test]
         python -m unittest discover -v greenlet.tests
         PATH="$OPATH" auditwheel repair dist/greenlet*.whl

setup.py

@@ -64,7 +64,7 @@
 
 if sys.platform == 'darwin' or 'clang' in plat_compiler:
     # The clang compiler doesn't use --std=c++11 by default
-    cpp_compile_args.append("--std=gnu++11")
+    cpp_compile_args.append("--std=c++11")
 elif is_win and "MSC" in plat_compiler:
     # Older versions of MSVC (Python 2.7) don't handle C++ exceptions
     # correctly by default. While newer versions do handle exceptions

src/greenlet/CObjects.cpp

@@ -15,7 +15,7 @@
 #include "greenlet_internal.hpp"
 #include "greenlet_refs.hpp"
 
-#include "greenlet_thread_state.hpp"
+
 #include "TThreadStateDestroy.cpp"
 
 #include "PyGreenlet.hpp"

src/greenlet/PyGreenlet.cpp

@@ -20,7 +20,7 @@ The Python slot functions for TGreenlet.
 
 #include "greenlet_refs.hpp"
 #include "greenlet_slp_switch.hpp"
-#include "greenlet_thread_state.hpp"
+
 #include "greenlet_thread_support.hpp"
 #include "TGreenlet.hpp"
 
@@ -48,22 +48,6 @@ using greenlet::BrokenGreenlet;
 using greenlet::ThreadState;
 using greenlet::PythonState;
 
-static PyGreenlet*
-green_create_main(ThreadState* state)
-{
-    PyGreenlet* gmain;
-
-    /* create the main greenlet for this thread */
-    gmain = (PyGreenlet*)PyType_GenericAlloc(&PyGreenlet_Type, 0);
-    if (gmain == NULL) {
-        Py_FatalError("green_create_main failed to alloc");
-        return NULL;
-    }
-    new MainGreenlet(gmain, state);
-
-    assert(Py_REFCNT(gmain) == 1);
-    return gmain;
-}
 
 
 static PyGreenlet*

src/greenlet/PyGreenlet.hpp

@@ -5,7 +5,7 @@
 #include "greenlet.h"
 #include "greenlet_compiler_compat.hpp"
 #include "greenlet_refs.hpp"
-#include "greenlet_thread_state.hpp"
+
 
 using greenlet::refs::OwnedGreenlet;
 using greenlet::refs::BorrowedGreenlet;
@@ -15,7 +15,6 @@ using greenlet::refs::PyErrPieces;
 
 
 // XXX: These doesn't really belong here, it's not a Python slot.
-static PyGreenlet* green_create_main(greenlet::ThreadState* state);
 static OwnedObject internal_green_throw(BorrowedGreenlet self, PyErrPieces& err_pieces);
 
 static PyGreenlet* green_new(PyTypeObject* type, PyObject* UNUSED(args), PyObject* UNUSED(kwds));

src/greenlet/PyGreenletUnswitchable.cpp

@@ -17,7 +17,7 @@
 // as well.
 #include "greenlet_refs.hpp"
 #include "greenlet_slp_switch.hpp"
-#include "greenlet_thread_state.hpp"
+
 #include "greenlet_thread_support.hpp"
 #include "TGreenlet.hpp"
 

src/greenlet/PyModule.cpp

@@ -3,7 +3,7 @@
 
 #include "greenlet_internal.hpp"
 
-#include "greenlet_thread_state.hpp"
+
 #include "TGreenletGlobals.cpp"
 #include "TMainGreenlet.cpp"
 #include "TThreadStateDestroy.cpp"

src/greenlet/TGreenlet.cpp

@@ -13,31 +13,31 @@
 #define TGREENLET_CPP
 #include "greenlet_internal.hpp"
 #include "TGreenlet.hpp"
-#include "greenlet_thread_state.hpp"
+
 
 #include "TGreenletGlobals.cpp"
 #include "TThreadStateDestroy.cpp"
 
 namespace greenlet {
 
 Greenlet::Greenlet(PyGreenlet* p)
+    :  Greenlet(p, StackState())
+{
+}
+
+Greenlet::Greenlet(PyGreenlet* p, const StackState& initial_stack)
+    :  _self(p), stack_state(initial_stack)
 {
-    p ->pimpl = this;
+    assert(p->pimpl == nullptr);
+    p->pimpl = this;
 }
 
 Greenlet::~Greenlet()
 {
     // XXX: Can't do this. tp_clear is a virtual function, and by the
     // time we're here, we've sliced off our child classes.
     //this->tp_clear();
-}
-
-Greenlet::Greenlet(PyGreenlet* p, const StackState& initial_stack)
-    : stack_state(initial_stack)
-{
-    // can't use a delegating constructor because of
-    // MSVC for Python 2.7
-    p->pimpl = this;
+    this->_self->pimpl = nullptr;
 }
 
 bool

src/greenlet/TGreenlet.hpp

@@ -152,7 +152,7 @@ namespace greenlet
         void set_new_cframe(_PyCFrame& frame) noexcept;
 #endif
 
-        inline void may_switch_away() noexcept;
+        void may_switch_away() noexcept;
         inline void will_switch_from(PyThreadState *const origin_tstate) noexcept;
         void did_finish(PyThreadState* tstate) noexcept;
     };
@@ -319,6 +319,7 @@ namespace greenlet
     {
     private:
         G_NO_COPIES_OF_CLS(Greenlet);
+        PyGreenlet* const _self;
     private:
         // XXX: Work to remove these.
         friend class ThreadState;
@@ -331,6 +332,8 @@ namespace greenlet
         PythonState python_state;
         Greenlet(PyGreenlet* p, const StackState& initial_state);
     public:
+        // This constructor takes ownership of the PyGreenlet, by
+        // setting ``p->pimpl = this;``.
         Greenlet(PyGreenlet* p);
         virtual ~Greenlet();
 
@@ -461,7 +464,10 @@ namespace greenlet
 
         // Return a borrowed greenlet that is the Python object
         // this object represents.
-        virtual BorrowedGreenlet self() const noexcept = 0;
+        inline BorrowedGreenlet self() const noexcept
+        {
+            return BorrowedGreenlet(this->_self);
+        }
 
         // For testing. If this returns true, we should pretend that
         // slp_switch() failed.
@@ -645,7 +651,6 @@ class TracingGuard
     {
     private:
         static greenlet::PythonAllocator<UserGreenlet> allocator;
-        BorrowedGreenlet _self;
         OwnedMainGreenlet _main_greenlet;
         OwnedObject _run_callable;
         OwnedGreenlet _parent;
@@ -674,7 +679,6 @@ class TracingGuard
 
         virtual const refs::BorrowedMainGreenlet main_greenlet() const;
 
-        virtual BorrowedGreenlet self() const noexcept;
         virtual void murder_in_place();
         virtual bool belongs_to_thread(const ThreadState* state) const;
         virtual int tp_traverse(visitproc visit, void* arg);
@@ -748,7 +752,6 @@ class TracingGuard
         virtual ThreadState* thread_state() const noexcept;
         void thread_state(ThreadState*) noexcept;
         virtual OwnedObject g_switch();
-        virtual BorrowedGreenlet self() const noexcept;
         virtual int tp_traverse(visitproc visit, void* arg);
     };
 

src/greenlet/TGreenletGlobals.cpp

@@ -15,7 +15,7 @@
 #include "greenlet_refs.hpp"
 #include "greenlet_exceptions.hpp"
 #include "greenlet_thread_support.hpp"
-#include "greenlet_thread_state.hpp"
+#include "greenlet_internal.hpp"
 
 namespace greenlet {
 

src/greenlet/TMainGreenlet.cpp

@@ -13,7 +13,7 @@
 #define T_MAIN_GREENLET_CPP
 
 #include "TGreenlet.hpp"
-#include "greenlet_thread_state.hpp"
+
 
 
 // Protected by the GIL. Incremented when we create a main greenlet,
@@ -63,12 +63,6 @@ MainGreenlet::thread_state(ThreadState* t) noexcept
     this->_thread_state = t;
 }
 
-BorrowedGreenlet
-MainGreenlet::self() const noexcept
-{
-    return BorrowedGreenlet(this->_self.borrow());
-}
-
 
 const BorrowedMainGreenlet
 MainGreenlet::main_greenlet() const