More testing.

src/greenlet/greenlet.cpp

@@ -1025,6 +1025,34 @@ Greenlet::slp_save_state(char *const stackref) noexcept
                                                 this->thread_state()->borrow_current()->stack_state);
 }
 
+OwnedObject
+Greenlet::on_switchstack_or_initialstub_failure(
+    Greenlet* target,
+    const Greenlet::switchstack_result_t& err,
+    const bool target_was_me,
+    const bool was_initial_stub)
+{
+    // If we get here, either g_initialstub()
+    // failed, or g_switchstack() failed. Either one of those
+    // cases SHOULD leave us in the original greenlet with a valid stack.
+    if (!PyErr_Occurred()) {
+        PyErr_SetString(
+            PyExc_SystemError,
+            was_initial_stub
+            ? "Failed to switch stacks into a greenlet for the first time."
+            : "Failed to switch stacks into a running greenlet.");
+    }
+    this->release_args();
+
+    if (target && !target_was_me) {
+        target->murder_in_place();
+    }
+
+    assert(!err.the_state_that_switched);
+    assert(!err.origin_greenlet);
+    return OwnedObject();
+
+}
 
 OwnedObject
 UserGreenlet::g_switch()
@@ -1059,8 +1087,8 @@ UserGreenlet::g_switch()
     while (target) {
         if (target->active()) {
             if (!target_was_me) {
-                target->args() <<= this->switch_args;
-                assert(!this->switch_args);
+                target->args() <<= this->args();
+                assert(!this->args());
             }
             err = target->g_switchstack();
             break;
@@ -1073,10 +1101,9 @@ UserGreenlet::g_switch()
             void* dummymarker;
             was_initial_stub = true;
             if (!target_was_me) {
-                target->args() <<= this->switch_args;
-                assert(!this->switch_args);
+                target->args() <<= this->args();
+                assert(!this->args());
             }
-
             try {
                 // This can only throw back to us while we're
                 // still in this greenlet. Once the new greenlet
@@ -1111,23 +1138,9 @@ UserGreenlet::g_switch()
     if (err.status < 0) {
         // If we get here, either g_initialstub()
         // failed, or g_switchstack() failed. Either one of those
-        // cases SHOULD leave us in the original greenlet with a valid stack.
-        if (!PyErr_Occurred()) {
-            PyErr_SetString(
-                PyExc_SystemError,
-                was_initial_stub
-                ? "Failed to switch stacks into a greenlet for the first time."
-                : "Failed to switch stacks into a running greenlet.");
-        }
-        this->release_args();
-
-        if (target && !target_was_me) {
-            target->murder_in_place();
-        }
-
-        assert(!err.the_state_that_switched);
-        assert(!err.origin_greenlet);
-        return OwnedObject();
+        // cases SHOULD leave us in the original greenlet with a valid
+        // stack.
+        return this->on_switchstack_or_initialstub_failure(target, err, target_was_me, was_initial_stub);
     }
 
     return err.the_state_that_switched->g_switch_finish(err);
@@ -1139,18 +1152,21 @@ MainGreenlet::g_switch()
     try {
         this->check_switch_allowed();
     }
-    catch(const PyErrOccurred&) {
+    catch (const PyErrOccurred&) {
         this->release_args();
         throw;
     }
 
     switchstack_result_t err = this->g_switchstack();
     if (err.status < 0) {
-        // XXX: This code path is untested.
-        assert(PyErr_Occurred());
-        assert(!err.the_state_that_switched);
-        assert(!err.origin_greenlet);
-        return OwnedObject();
+        // XXX: This code path is untested, but it is shared
+        // with the UserGreenlet path that is tested.
+        return this->on_switchstack_or_initialstub_failure(
+            this,
+            err,
+            true, // target was me
+            false // was initial stub
+        );
     }
 
     return err.the_state_that_switched->g_switch_finish(err);
@@ -1185,7 +1201,7 @@ UserGreenlet::g_initialstub(void* mark)
     // We'll restore them when we return in that case.
     // Scope them tightly to avoid ref leaks.
     {
-        SwitchingArgs args(this->switch_args);
+        SwitchingArgs args(this->args());
 
         /* save exception in case getattr clears it */
         PyErrPieces saved;
@@ -1211,10 +1227,10 @@ UserGreenlet::g_initialstub(void* mark)
          */
         if (this->stack_state.started()) {
             // the successful switch cleared these out, we need to
-            // restore our version.
-            assert(!this->switch_args);
-            this->switch_args <<= args;
-
+            // restore our version. They will be copied on up to the
+            // next target.
+            assert(!this->args());
+            this->args() <<= args;
             throw GreenletStartedWhileInPython();
         }
     }
@@ -1345,8 +1361,8 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& _run)
     // could switch back to us, so we need to grab the
     // arguments locally.
     SwitchingArgs args;
-    args <<= this->switch_args;
-    assert(!this->switch_args);
+    args <<= this->args();
+    assert(!this->args());
 
     // The first switch we need to manually call the trace
     // function here instead of in g_switch_finish, because we
@@ -1435,12 +1451,12 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& _run)
 
     if (!result
         && mod_globs->PyExc_GreenletExit.PyExceptionMatches()
-        && (this->switch_args)) {
+        && (this->args())) {
         // This can happen, for example, if our only reference
         // goes away after we switch back to the parent.
         // See test_dealloc_switch_args_not_lost
         PyErrPieces clear_error;
-        result <<= this->switch_args;
+        result <<= this->args();
         result = single_result(result);
     }
     this->release_args();

src/greenlet/greenlet_exceptions.hpp

@@ -21,6 +21,11 @@ namespace greenlet {
             assert(PyErr_Occurred());
         }
 
+        PyErrOccurred(const std::string& msg) : std::runtime_error(msg)
+        {
+            assert(PyErr_Occurred());
+        }
+
         PyErrOccurred(PyObject* exc_kind, const char* const msg)
             : std::runtime_error(msg)
         {
@@ -81,10 +86,10 @@ namespace greenlet {
     };
 
     static inline PyObject*
-    Require(PyObject* p)
+    Require(PyObject* p, const std::string& msg="")
     {
         if (!p) {
-            throw PyErrOccurred();
+            throw PyErrOccurred(msg);
         }
         return p;
     };

src/greenlet/greenlet_greenlet.hpp

@@ -463,6 +463,12 @@ namespace greenlet
             }
         };
 
+        OwnedObject on_switchstack_or_initialstub_failure(
+            Greenlet* target,
+            const switchstack_result_t& err,
+            const bool target_was_me=false,
+            const bool was_initial_stub=false);
+
         // Returns the previous greenlet we just switched away from.
         virtual OwnedGreenlet g_switchstack_success() noexcept;
 

src/greenlet/greenlet_refs.hpp

@@ -80,6 +80,7 @@ namespace greenlet
         typedef OwnedReference<PyObject, NoOpChecker> OwnedObject;
 
         class ImmortalObject;
+        class ImmortalString;
 
         template<typename T, TypeChecker TC>
         class _OwnedGreenlet;
@@ -222,7 +223,7 @@ namespace greenlet {
         inline const std::string as_str() const noexcept;
         inline OwnedObject PyGetAttr(const ImmortalObject& name) const noexcept;
         inline OwnedObject PyRequireAttr(const char* const name) const;
-        inline OwnedObject PyRequireAttr(const ImmortalObject& name) const;
+        inline OwnedObject PyRequireAttr(const ImmortalString& name) const;
         inline OwnedObject PyCall(const BorrowedObject& arg) const;
         inline OwnedObject PyCall(PyGreenlet* arg) const ;
         inline OwnedObject PyCall(PyObject* arg) const ;
@@ -650,6 +651,11 @@ namespace greenlet {
             return *this;
         }
 
+        inline operator std::string() const
+        {
+            return this->str;
+        }
+
     };
 
     template<typename T, TypeChecker TC>
@@ -689,16 +695,20 @@ namespace greenlet {
     inline OwnedObject PyObjectPointer<T, TC>::PyRequireAttr(const char* const name) const
     {
         assert(this->p);
-        return OwnedObject::consuming(Require(PyObject_GetAttrString(this->p, name)));
+        return OwnedObject::consuming(Require(PyObject_GetAttrString(this->p, name), name));
     }
 
     template<typename T, TypeChecker TC>
-    inline OwnedObject PyObjectPointer<T, TC>::PyRequireAttr(const ImmortalObject& name) const
+    inline OwnedObject PyObjectPointer<T, TC>::PyRequireAttr(const ImmortalString& name) const
     {
         assert(this->p);
         return OwnedObject::consuming(Require(
-                   PyObject_GetAttr(reinterpret_cast<PyObject*>(this->p),
-                                    name)));
+                   PyObject_GetAttr(
+                      reinterpret_cast<PyObject*>(this->p),
+                      name
+                   ),
+                   name
+               ));
     }
 
     template<typename T, TypeChecker TC>

src/greenlet/tests/fail_initialstub_already_started.py

@@ -0,0 +1,78 @@
+"""
+Testing initialstub throwing an already started exception.
+"""
+
+import greenlet
+
+a = None
+b = None
+c = None
+main = greenlet.getcurrent()
+
+# If we switch into a dead greenlet,
+# we go looking for its parents.
+# if a parent is not yet started, we start it.
+
+results = []
+
+def a_run(*args):
+    #results.append('A')
+    results.append(('Begin A', args))
+
+
+def c_run():
+    results.append('Begin C')
+    b.switch('From C')
+    results.append('C done')
+
+class A(greenlet.greenlet): pass
+
+class B(greenlet.greenlet):
+    doing_it = False
+    def __getattribute__(self, name):
+        if name == 'run' and not self.doing_it:
+            assert greenlet.getcurrent() is c
+            self.doing_it = True
+            results.append('Switch to b from B.__getattribute__ in '
+                           + type(greenlet.getcurrent()).__name__)
+            b.switch()
+            results.append('B.__getattribute__ back from main in '
+                           + type(greenlet.getcurrent()).__name__)
+        if name == 'run':
+            name = '_B_run'
+        return object.__getattribute__(self, name)
+
+    def _B_run(self, *arg):
+        results.append(('Begin B', arg))
+        results.append('_B_run switching to main')
+        main.switch('From B')
+
+class C(greenlet.greenlet):
+    pass
+a = A(a_run)
+b = B(parent=a)
+c = C(c_run, b)
+
+# Start a child; while running, it will start B,
+# but starting B will ALSO start B.
+result = c.switch()
+results.append(('main from c', result))
+
+# Switch back to C, which was in the middle of switching
+# already. This will throw the ``GreenletStartedWhileInPython``
+# exception, which results in parent A getting started (B is finished)
+c.switch()
+
+results.append(('A dead?', a.dead, 'B dead?', b.dead, 'C dead?', c.dead))
+
+# A and B should both be dead now.
+assert a.dead
+assert b.dead
+assert not c.dead
+
+result = c.switch()
+results.append(('main from c.2', result))
+# Now C is dead
+assert c.dead
+
+print("RESULTS:", results)

src/greenlet/tests/test_greenlet.py

@@ -1225,6 +1225,18 @@ def test_reentrant_switch_three_greenlets2(self):
             output
         )
 
+    def test_reentrant_switch_GreenletAlreadyStartedInPython(self):
+        output = self.run_script('fail_initialstub_already_started.py')
+
+        self.assertIn(
+            "RESULTS: ['Begin C', 'Switch to b from B.__getattribute__ in C', "
+            "('Begin B', ()), '_B_run switching to main', ('main from c', 'From B'), "
+            "'B.__getattribute__ back from main in C', ('Begin A', (None,)), "
+            "('A dead?', True, 'B dead?', True, 'C dead?', False), "
+            "'C done', ('main from c.2', None)]",
+            output
+        )
+
 if __name__ == '__main__':
     import unittest
     unittest.main()