context: Fix deadlock when new context is created from already-canceled parent

When _BaseCtx is setting up cancel propagation it locks a parent,
checks for parent.err != nil, and, if it is, calls
ctx._cancel(parent.err) _with_ _holding_ parent.mu. Since _cancel
internally also goes through parents and locks them, this was deadlocking
on the second call to parent.mu.lock().

-> Fix it by calling ctx._cancel(err) in the constructor outside of
parent lock.

The bug was there from the beginning - from e9567c7b (context: New
package that mirrors Go's context).

/trusted-by @jerome
/reviewed-on nexedi/pygolang!16

golang/context.cpp

@@ -118,12 +118,14 @@ struct _BaseCtx : _Context, object {
             // parent is cancellable - glue to propagate cancel from it to us
             _BaseCtx *_parent = dynamic_cast<_BaseCtx *>(parent._ptr());
             if (_parent != nil) {
+                error err = nil;
                 _parent->_mu.lock();
-                    if (_parent->_err != nil)
-                        ctx._cancel(_parent->_err);
-                    else
+                    err = _parent->_err;
+                    if (err == nil)
                         _parent->_children.insert(bctx);
                 _parent->_mu.unlock();
+                if (err != nil)
+                    ctx._cancel(err);
             }
             else {
                 if (_ready(pdone))

golang/context_test.py

 # -*- coding: utf-8 -*-
-# Copyright (C) 2019  Nexedi SA and Contributors.
-#                     Kirill Smelkov <kirr@nexedi.com>
+# Copyright (C) 2019-2021  Nexedi SA and Contributors.
+#                          Kirill Smelkov <kirr@nexedi.com>
 #
 # This program is free software: you can Use, Study, Modify and Redistribute
 # it under the terms of the GNU General Public License version 3, or (at your
@@ -265,6 +265,43 @@ def test_deadline():
     assertCtx(ctx,  Z,  deadline=d, err=D, done=Y)
 
 
+# test_already_canceled verifies context creation from already canceled parent.
+# this used to deadlock.
+def test_already_canceled():
+    parent, pcancel = context.with_cancel(bg)
+    assertCtx(parent, Z)
+    pcancel()
+    assertCtx(parent, Z, err=C, done=Y)
+
+    ctxC, _ = context.with_cancel(parent)
+    assert ctxC.done() != parent.done()
+    assertCtx(parent, Z, err=C, done=Y)  # no ctxC in children
+    assertCtx(ctxC,   Z, err=C, done=Y)
+
+    ctxT, _ = context.with_timeout(parent, 10*dt)
+    d = ctxT.deadline()
+    assert ctxT.done() != parent.done()
+    assertCtx(parent, Z, err=C, done=Y)  # no ctxT in children
+    assertCtx(ctxT,   Z, deadline=d, err=C, done=Y)
+
+    d = time.now() + 10*dt
+    ctxD, _ = context.with_deadline(parent, d)
+    assert ctxD.done() != parent.done()
+    assertCtx(parent, Z, err=C, done=Y)  # no ctxD in children
+    assertCtx(ctxD,   Z, deadline=d, err=C, done=Y)
+
+    ctxM, _ = context.merge(parent, bg)
+    assert ctxM.done() != parent.done()
+    assertCtx(parent, Z, err=C, done=Y)  # no ctxM in children
+    assertCtx(ctxM,   Z, err=C, done=Y)
+
+    ctxV    = context.with_value(parent, kHello, "world")
+    assert ctxV.done() == parent.done()
+    assert ctxV.value(kHello) == "world"
+    assertCtx(parent, Z, err=C, done=Y)  # no ctxV in children
+    assertCtx(ctxV,   Z, err=C, done=Y)
+
+
 # ---- misc ----
 
 # _ready returns whether channel ch is ready.