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.
This commit is contained in:
Markus Mäkelä
2019-01-16 21:03:19 +02:00
parent 519cd7d889
commit a469ef83b6
2 changed files with 31 additions and 35 deletions

View File

@ -3612,8 +3612,8 @@ int poll_add_dcb(DCB* dcb)
} }
else else
{ {
// Round-robin the client connection worker assignment // Assign to current worker
owner = RoutingWorker::pick_worker(); owner = RoutingWorker::get_current();
} }
new_state = DCB_STATE_POLLING; new_state = DCB_STATE_POLLING;

View File

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