Remove several uses of G_NOEXCEPT because they were innacurate.

Turns out on Linux with gcc and libstdc++ calling pthread_exit() raises an uncatchable exception. That is called when reacquiring the GIL in a background thread if the main thread has terminated. The use of G_NOEXCEPT in that code path was resulting in the C++ runtime calling std::terminate when that happened.

6 files changed, 61 insertions, 13 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index b2d69b9..49bfaf9 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,7 +5,9 @@
 2.0.0rc4 (unreleased)
 =====================
 
-- Nothing changed yet.
+- Linux: Fix a rare crash that could occur when shutting down an
+  interpreter running multiple threads, when some of those threads are
+  in greenlets making calls to functions that release the GIL.
 
 
 2.0.0rc3 (2022-10-29)
diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp
index 4e46d42..ed8acf5 100644
--- a/src/greenlet/greenlet.cpp
+++ b/src/greenlet/greenlet.cpp
@@ -1296,7 +1296,7 @@ UserGreenlet::g_initialstub(void* mark)
 
 
 void
-UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) G_NOEXCEPT
+UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run)
 {
     // The arguments here would be another great place for move.
     // As it is, we take them as a reference so that when we clear
@@ -1385,9 +1385,19 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run)
             //
             // Hopefully the basic C stdlib is still functional enough
             // for us to at least print an error.
+            //
+            // It gets more complicated than that, though, on some
+            // platforms, specifically at least Linux/gcc/libstdc++. They use
+            // an exception to unwind the stack when a background
+            // thread exits. (See comments about G_NOEXCEPT.) So this
+            // may not actually represent anything untoward.
+# if defined(WIN32) || defined(_WIN32)
             fprintf(stderr, "greenlet: Unhandled C++ exception from a greenlet run function. ");
             fprintf(stderr, "Because memory is likely corrupted, terminating process.");
             abort();
+#else
+            throw;
+#endif
         }
     }
     args.CLEAR();
diff --git a/src/greenlet/greenlet_compiler_compat.hpp b/src/greenlet/greenlet_compiler_compat.hpp
index bf11813..779ccf5 100644
--- a/src/greenlet/greenlet_compiler_compat.hpp
+++ b/src/greenlet/greenlet_compiler_compat.hpp
@@ -5,6 +5,35 @@
 /**
  * Definitions to aid with compatibility with different compilers.
  *
+ * .. caution:: Use extreme care with G_NOEXCEPT.
+ * Some compilers and runtimes, specifically gcc/libgcc/libstdc++ on
+ * Linux, implement stack unwinding by throwing an uncatchable
+ * exception, one that specifically does not appear to be an active
+ * exception to the rest of the runtime. If this happens while we're in a G_NOEXCEPT function,
+ * we have violated our dynamic exception contract, and so the runtime
+ * will call std::terminate(), which kills the process with the
+ * unhelpful message "terminate called without an active exception".
+ *
+ * This has happened in this scenario: A background thread is running
+ * a greenlet that has made a native call and released the GIL.
+ * Meanwhile, the main thread finishes and starts shutting down the
+ * interpreter. When the background thread is scheduled again and
+ * attempts to obtain the  GIL, it notices that the interpreter is
+ * exiting and calls ``pthread_exit()``. This in turn starts to unwind
+ * the stack by throwing that exception. But we had the ``PyCall``
+ * functions annotated as G_NOEXCEPT, so the runtime terminated us.
+ *
+ * #2  0x00007fab26fec2b7 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6
+ * #3  0x00007fab26febb3c in __gxx_personality_v0 () from /lib/x86_64-linux-gnu/libstdc++.so.6
+ * #4  0x00007fab26f34de6 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
+ * #6  0x00007fab276a34c6 in __GI___pthread_unwind  at ./nptl/unwind.c:130
+ * #7  0x00007fab2769bd3a in __do_cancel () at ../sysdeps/nptl/pthreadP.h:280
+ * #8  __GI___pthread_exit (value=value@entry=0x0) at ./nptl/pthread_exit.c:36
+ * #9  0x000000000052e567 in PyThread_exit_thread () at ../Python/thread_pthread.h:370
+ * #10 0x00000000004d60b5 in take_gil at ../Python/ceval_gil.h:224
+ * #11 0x00000000004d65f9 in PyEval_RestoreThread  at ../Python/ceval.c:467
+ * #12 0x000000000060cce3 in setipaddr  at ../Modules/socketmodule.c:1203
+ * #13 0x00000000006101cd in socket_gethostbyname
  */
 
 
diff --git a/src/greenlet/greenlet_greenlet.hpp b/src/greenlet/greenlet_greenlet.hpp
index 1b8a83f..f9a1b83 100644
--- a/src/greenlet/greenlet_greenlet.hpp
+++ b/src/greenlet/greenlet_greenlet.hpp
@@ -589,7 +589,7 @@ namespace greenlet
     protected:
         virtual switchstack_result_t g_initialstub(void* mark);
     private:
-        void inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) G_NOEXCEPT;
+        void inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run);
     };
 
     class MainGreenlet : public Greenlet
diff --git a/src/greenlet/greenlet_refs.hpp b/src/greenlet/greenlet_refs.hpp
index 622880b..160b9e4 100644
--- a/src/greenlet/greenlet_refs.hpp
+++ b/src/greenlet/greenlet_refs.hpp
@@ -223,14 +223,14 @@ namespace greenlet {
         inline OwnedObject PyGetAttr(const ImmortalObject& name) const G_NOEXCEPT;
         inline OwnedObject PyRequireAttr(const char* const name) const;
         inline OwnedObject PyRequireAttr(const ImmortalObject& name) const;
-        inline OwnedObject PyCall(const BorrowedObject& arg) const G_NOEXCEPT;
-        inline OwnedObject PyCall(PyGreenlet* arg) const G_NOEXCEPT;
-        inline OwnedObject PyCall(const PyObject* arg) const G_NOEXCEPT;
+        inline OwnedObject PyCall(const BorrowedObject& arg) const;
+        inline OwnedObject PyCall(PyGreenlet* arg) const ;
+        inline OwnedObject PyCall(const PyObject* arg) const ;
         // PyObject_Call(this, args, kwargs);
         inline OwnedObject PyCall(const BorrowedObject args,
-                                  const BorrowedObject kwargs) const G_NOEXCEPT;
+                                  const BorrowedObject kwargs) const;
         inline OwnedObject PyCall(const OwnedObject& args,
-                                  const OwnedObject& kwargs) const G_NOEXCEPT;
+                                  const OwnedObject& kwargs) const;
 
     protected:
         void _set_raw_pointer(void* t)
@@ -706,19 +706,19 @@ namespace greenlet {
     }
 
     template<typename T, TypeChecker TC>
-    inline OwnedObject PyObjectPointer<T, TC>::PyCall(const BorrowedObject& arg) const G_NOEXCEPT
+    inline OwnedObject PyObjectPointer<T, TC>::PyCall(const BorrowedObject& arg) const
     {
         return this->PyCall(arg.borrow());
     }
 
     template<typename T, TypeChecker TC>
-    inline OwnedObject PyObjectPointer<T, TC>::PyCall(PyGreenlet* arg) const G_NOEXCEPT
+    inline OwnedObject PyObjectPointer<T, TC>::PyCall(PyGreenlet* arg) const
     {
         return this->PyCall(reinterpret_cast<const PyObject*>(arg));
     }
 
     template<typename T, TypeChecker TC>
-    inline OwnedObject PyObjectPointer<T, TC>::PyCall(const PyObject* arg) const G_NOEXCEPT
+    inline OwnedObject PyObjectPointer<T, TC>::PyCall(const PyObject* arg) const
     {
         assert(this->p);
         return OwnedObject::consuming(PyObject_CallFunctionObjArgs(this->p, arg, NULL));
@@ -726,7 +726,7 @@ namespace greenlet {
 
     template<typename T, TypeChecker TC>
     inline OwnedObject PyObjectPointer<T, TC>::PyCall(const BorrowedObject args,
-                                                  const BorrowedObject kwargs) const G_NOEXCEPT
+                                                  const BorrowedObject kwargs) const
     {
         assert(this->p);
         return OwnedObject::consuming(PyObject_Call(this->p, args, kwargs));
@@ -734,7 +734,7 @@ namespace greenlet {
 
     template<typename T, TypeChecker TC>
     inline OwnedObject PyObjectPointer<T, TC>::PyCall(const OwnedObject& args,
-                                                  const OwnedObject& kwargs) const G_NOEXCEPT
+                                                  const OwnedObject& kwargs) const
     {
         assert(this->p);
         return OwnedObject::consuming(PyObject_Call(this->p, args.borrow(), kwargs.borrow()));
diff --git a/src/greenlet/tests/_test_extension_cpp.cpp b/src/greenlet/tests/_test_extension_cpp.cpp
index d9d7f38..344929d 100644
--- a/src/greenlet/tests/_test_extension_cpp.cpp
+++ b/src/greenlet/tests/_test_extension_cpp.cpp
@@ -115,9 +115,16 @@ test_exception_switch_and_do_in_g2(PyObject* self, PyObject* args)
          * might crash before below py-level exception might become printed.
          * -> print something to stderr to make it clear that we had entered
          *    this catch block.
+         * See comments in inner_bootstrap()
          */
+#if defined(WIN32) || defined(_WIN32)
         fprintf(stderr, "C++ exception unexpectedly caught in g1\n");
         PyErr_SetString(PyExc_AssertionError, "C++ exception unexpectedly caught in g1");
+        Py_XDECREF(result);
+        return NULL;
+#else
+        throw;
+#endif
     }
 
     Py_XDECREF(result);