From a469ef83b64f10f82e3db5134b73ea5416e91b1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Wed, 16 Jan 2019 21:03:19 +0200
Subject: [PATCH] MXS-2217: Pick DCB owner before adding to epoll

There is a race condition between the addition of the DCB into epoll and
the execution of the event that initiates the protocol pointer for the DCB
and sends the handshake to the client. If a hangup event would occur
before the handshake would be sent, it would be possible that the DCB
would get freed before the code that sends the handshake is executed.

By picking the worker who owns the DCB before the DCB is placed into the
owner's epoll instance, we make sure no events arrive on the DCB while the
control is transferred from the accepting worker to the owning
worker.
---
 server/core/dcb.cc                            |  4 +-
 .../MySQL/mariadbclient/mysql_client.cc       | 62 +++++++++----------
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/server/core/dcb.cc b/server/core/dcb.cc
index c592dec8c..eb07a7ad7 100644
--- a/server/core/dcb.cc
+++ b/server/core/dcb.cc
@@ -3612,8 +3612,8 @@ int poll_add_dcb(DCB* dcb)
         }
         else
         {
-            // Round-robin the client connection worker assignment
-            owner = RoutingWorker::pick_worker();
+            // Assign to current worker
+            owner = RoutingWorker::get_current();
         }
 
         new_state = DCB_STATE_POLLING;
diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc
index ae2db1a76..c1302ed0c 100644
--- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc
+++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc
@@ -1435,44 +1435,40 @@ int gw_MySQLAccept(DCB* listener)
 static void gw_process_one_new_client(DCB* client_dcb)
 {
     /**
-     * Set new descriptor to event set. At the same time,
-     * change state to DCB_STATE_POLLING so that
-     * thread which wakes up sees correct state.
+     * The worker who owns the DCB is chosen here, before any epoll events for it can be processed.
+     * This guarantees that the first event for the DCB is processed only after the following
+     * task has been processed by the owning thread.
      */
-    if (poll_add_dcb(client_dcb) == -1)
-    {
-        /* Send a custom error as MySQL command reply */
-        mysql_send_custom_error(client_dcb,
-                                1,
-                                0,
-                                "MaxScale encountered system limit while "
-                                "attempting to register on an epoll instance.");
+    mxs::RoutingWorker* worker = mxs::RoutingWorker::pick_worker();
 
-        /** close client_dcb */
-        dcb_close(client_dcb);
+    worker->execute([=]() {
+                        client_dcb->protocol = mysql_protocol_init(client_dcb, client_dcb->fd);
+                        MXS_ABORT_IF_NULL(client_dcb->protocol);
 
-        /** Previous state is recovered in poll_add_dcb. */
-        MXS_ERROR("Failed to add dcb %p for fd %d to epoll set.",
-                  client_dcb,
-                  client_dcb->fd);
-        return;
-    }
-    else
-    {
-        // Move the rest of the initialization process to the owning worker
-        mxs::RoutingWorker* worker = static_cast<mxs::RoutingWorker*>(client_dcb->poll.owner);
+                        /**
+                         * Set new descriptor to event set. At the same time,
+                         * change state to DCB_STATE_POLLING so that
+                         * thread which wakes up sees correct state.
+                         */
+                        if (poll_add_dcb(client_dcb) == -1)
+                        {
+                            /* Send a custom error as MySQL command reply */
+                            mysql_send_custom_error(client_dcb, 1, 0,
+                                                    "MaxScale encountered system limit while "
+                                                    "attempting to register on an epoll instance.");
 
-        worker->execute([=](){
-            client_dcb->protocol = mysql_protocol_init(client_dcb, client_dcb->fd);
-            MXS_ABORT_IF_NULL(client_dcb->protocol);
-            MySQLSendHandshake(client_dcb);
-        }, mxs::RoutingWorker::EXECUTE_AUTO);
+                            /** close client_dcb */
+                            dcb_close(client_dcb);
 
-        MXS_DEBUG("Added dcb %p for fd %d to epoll set.",
-                  client_dcb,
-                  client_dcb->fd);
-    }
-    return;
+                            /** Previous state is recovered in poll_add_dcb. */
+                            MXS_ERROR("Failed to add dcb %p for fd %d to epoll set.",
+                                      client_dcb, client_dcb->fd);
+                        }
+                        else
+                        {
+                            MySQLSendHandshake(client_dcb);
+                        }
+                    }, mxs::RoutingWorker::EXECUTE_AUTO);
 }
 
 static int gw_error_client_event(DCB* dcb)