Add some comments to our use of Boost context

diff --git a/pdns/mtasker_fcontext.cc b/pdns/mtasker_fcontext.cc
index a6a9d177c4f7638ae3f304d33529265464c544a6..bd6a4898ba1e485acaa7cbffe71148f4c76588aa 100644 (file)
--- a/pdns/mtasker_fcontext.cc
+++ b/pdns/mtasker_fcontext.cc
@@ -8,11 +8,23 @@
 using boost::context::make_fcontext;
 
 #if BOOST_VERSION < 105600
+/* Note: This typedef means functions taking fcontext_t*, like jump_fcontext(),
+ * now require a reinterpret_cast rather than a static_cast, since we're
+ * casting from pdns_context_t->uc_mcontext, which is void**, to
+ * some_opaque_struct**. In later versions fcontext_t is already void*. So if
+ * you remove this, then fix the ugly.
+ */
 using fcontext_t = boost::context::fcontext_t*;
 
+/* Emulate the >= 1.56 API for Boost 1.52 through 1.55 */
 static inline intptr_t
 jump_fcontext (fcontext_t* const ofc, fcontext_t const nfc, 
                intptr_t const arg) {
+    /* If the fcontext_t is preallocated then use it, otherwise allocate one
+     * on the stack ('self') and stash a pointer away in *ofc so the returning
+     * MThread can access it. This is safe because we're suspended, so the
+     * context object always outlives the jump.
+     */
     if (*ofc) {
         return boost::context::jump_fcontext (*ofc, nfc, arg);
     } else {
@@ -31,6 +43,9 @@ static_assert (std::is_pointer<fcontext_t>::value,
                "Boost Context has changed the fcontext_t type again :-(");
 #endif
 
+/* Boost context only provides a means of passing a single argument across a
+ * jump. args_t simply provides a way to pass more by reference.
+ */
 struct args_t {
     fcontext_t prev_ctx = nullptr;
     pdns_ucontext_t* self = nullptr;
@@ -41,6 +56,15 @@ extern "C" {
 static
 void
 threadWrapper (intptr_t const xargs) {
+    /* Access the args passed from pdns_makecontext, and copy them directly from
+     * the calling stack on to ours (we're now using the MThreads stack).
+     * This saves heap allocating an args object, at the cost of an extra
+     * context switch to fashion this constructor-like init phase. The work
+     * function object is still only moved after we're (re)started, so may
+     * still be set or changed after a call to pdns_makecontext. This matches
+     * the behaviour of the System V implementation, which can inherently only
+     * be passed ints and pointers.
+     */
     auto args = reinterpret_cast<args_t*>(xargs);
     auto ctx = args->self;
     auto work = args->work;
@@ -55,6 +79,7 @@ threadWrapper (intptr_t const xargs) {
         ctx->exception = std::current_exception();
     }
 
+    /* Emulate the System V uc_link feature. */
     auto const next_ctx = ctx->uc_link->uc_mcontext;
     jump_fcontext (reinterpret_cast<fcontext_t*>(&ctx->uc_mcontext),
                    static_cast<fcontext_t>(next_ctx),
@@ -71,6 +96,9 @@ pdns_ucontext_t::pdns_ucontext_t
 
 pdns_ucontext_t::~pdns_ucontext_t
 () {
+    /* There's nothing to delete here since fcontext doesn't require anything
+     * to be heap allocated.
+     */
 }
 
 void