Skip to content

Commit

Permalink
Bug #30771233: MYSQL_REAL_CONNECT_NONBLOCKING() IS NOT NONBLOCKING
Browse files Browse the repository at this point in the history 
RB#23860

mysql_real_connect_nonblocking() had a busy loop in it that was
basically turning it into a blocking function.
Removed the busy loop.
Added a new state for connect to track between connect attempted and
connect complete.
Fixed the test cases that were assuming that
mysql_real_connect_nonblocking was:
1. never returning NET_ASYNC_NOT_READY
2. when it was returning that there are always data to be waited on the
socket. This was dead code test that is now alive.

No test suite added since the existing suite already covers this.
  • Loading branch information
@gkodinov
gkodinov committed Feb 14, 2020
1 parent 009da50 commit 1923bb7
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 23 deletions.
3 changes: 0 additions & 3 deletions client/mysqltest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -861,9 +861,6 @@ static MYSQL *async_mysql_real_connect_wrapper(
mysql, host, user, passwd, db, port, unix_socket, client_flag)) ==
NET_ASYNC_NOT_READY) {
t.check();
NET_ASYNC *net_async = NET_ASYNC_DATA(&(mysql->net));
socket_event_listen(net_async->async_blocking_state,
mysql_get_socket_descriptor(mysql));
}
if (status == NET_ASYNC_ERROR)
return nullptr;
Expand Down
5 changes: 3 additions & 2 deletions include/violite.h
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
/* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License, version 2.0,
Expand Down Expand Up @@ -199,7 +199,8 @@ ssize_t vio_pending(MYSQL_VIO vio);
int vio_timeout(MYSQL_VIO vio, uint which, int timeout_sec);
/* Connect to a peer. */
bool vio_socket_connect(MYSQL_VIO vio, struct sockaddr *addr, socklen_t len,
bool nonblocking, int timeout);
bool nonblocking, int timeout,
bool *connect_done = nullptr);

bool vio_get_normalized_ip_string(const struct sockaddr *addr,
size_t addr_length, char *ip_string,
Expand Down
86 changes: 79 additions & 7 deletions sql-common/client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4455,6 +4455,7 @@ static mysql_state_machine_status authsm_finish_auth(mysql_async_auth *ctx);
static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx);
static mysql_state_machine_status csm_complete_connect(
mysql_async_connect *ctx);
static mysql_state_machine_status csm_wait_connect(mysql_async_connect *ctx);
static mysql_state_machine_status csm_read_greeting(mysql_async_connect *ctx);
static mysql_state_machine_status csm_parse_handshake(mysql_async_connect *ctx);
static mysql_state_machine_status csm_establish_ssl(mysql_async_connect *ctx);
Expand Down Expand Up @@ -5664,17 +5665,15 @@ net_async_status STDCALL mysql_real_connect_nonblocking(
ASYNC_DATA(mysql)->async_op_status = ASYNC_OP_CONNECT;
}

do {
status = ctx->state_function(ctx);
} while (status != STATE_MACHINE_FAILED && status != STATE_MACHINE_DONE);
status = ctx->state_function(ctx);

if (status == STATE_MACHINE_DONE) {
my_free(ASYNC_DATA(mysql)->connect_context);
ASYNC_DATA(mysql)->connect_context = nullptr;
ASYNC_DATA(mysql)->async_op_status = ASYNC_OP_UNSET;
return NET_ASYNC_COMPLETE;
}
{
if (status == STATE_MACHINE_FAILED) {
DBUG_PRINT("error", ("message: %u/%s (%s)", mysql->net.last_errno,
mysql->net.sqlstate, mysql->net.last_error));
/* Free alloced memory */
Expand All @@ -5689,6 +5688,7 @@ net_async_status STDCALL mysql_real_connect_nonblocking(
my_free(ctx);
return NET_ASYNC_ERROR;
}
return NET_ASYNC_NOT_READY;
}
/**
Begin the connection to the server, including any DNS resolution
Expand All @@ -5703,6 +5703,8 @@ static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx) {
uint port = ctx->port;
const char *unix_socket = ctx->unix_socket;
ulong client_flag = ctx->client_flag;
bool connect_done =
true; // this is true for most of the connect methods except sockets

DBUG_TRACE;

Expand Down Expand Up @@ -5835,7 +5837,7 @@ static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx) {

if (vio_socket_connect(net->vio, (struct sockaddr *)&UNIXaddr,
sizeof(UNIXaddr), ctx->non_blocking,
get_vio_connect_timeout(mysql))) {
get_vio_connect_timeout(mysql), &connect_done)) {
DBUG_PRINT("error",
("Got error %d on connect to local server", socket_errno));
set_mysql_extended_error(mysql, CR_CONNECTION_ERROR, unknown_sqlstate,
Expand Down Expand Up @@ -6023,7 +6025,7 @@ static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx) {

status = vio_socket_connect(
net->vio, t_res->ai_addr, (socklen_t)t_res->ai_addrlen,
ctx->non_blocking, get_vio_connect_timeout(mysql));
ctx->non_blocking, get_vio_connect_timeout(mysql), &connect_done);
/*
Here we rely on vio_socket_connect() to return success only if
the connect attempt was really successful. Otherwise we would
Expand Down Expand Up @@ -6063,7 +6065,7 @@ static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx) {
}
}

ctx->state_function = csm_complete_connect;
ctx->state_function = connect_done ? csm_complete_connect : csm_wait_connect;
ctx->host = host;
ctx->user = user;
ctx->passwd = passwd;
Expand All @@ -6074,6 +6076,76 @@ static mysql_state_machine_status csm_begin_connect(mysql_async_connect *ctx) {
return STATE_MACHINE_CONTINUE;
}

/**
Wait for async connect attempt to complete.
*/
static mysql_state_machine_status csm_wait_connect(mysql_async_connect *ctx) {
NET *net = &(ctx->mysql->net);
MYSQL_VIO vio = net->vio;
int timeout_ms = 1; // this is 1ms: the smallest non-zero timeout we can use
int ret;

DBUG_TRACE;

DBUG_PRINT(
"enter",
("host: %s db: %s user: %s (client)", ctx->host ? ctx->host : "(Null)",
ctx->db ? ctx->db : "(Null)", ctx->user ? ctx->user : "(Null)"));

if (!net->vio) {
DBUG_PRINT("error", ("Unknown protocol %d", ctx->mysql->options.protocol));
set_mysql_error(ctx->mysql, CR_CONN_UNKNOW_PROTOCOL, unknown_sqlstate);
return STATE_MACHINE_FAILED;
}

/*
The connect() is in progress. The vio_io_wait() with the smallest non-zero
timeout possible can be used to peek if connect() completed.
If vio_io_wait() returns 0,
the socket never became writable and we'll return to caller.
Otherwise, if vio_io_wait() returns 1, then one of two conditions
exist:
1. An error occurred. Use getsockopt() to check for this.
2. The connection was set up successfully: getsockopt() will
return 0 as an error.
*/
if (vio_io_wait(vio, VIO_IO_EVENT_CONNECT, timeout_ms) == 1) {
int error;
IF_WIN(int, socklen_t) optlen = sizeof(error);
IF_WIN(char, void) *optval = (IF_WIN(char, void) *)&error;

/*
At this point, we know that something happened on the socket.
But this does not means that everything is alright. The connect
might have failed. We need to retrieve the error code from the
socket layer. We must return success only if we are sure that
it was really a success. Otherwise we might prevent the caller
from trying another address to connect to.
*/
DBUG_PRINT("info", ("Connect to '%s' completed", ctx->host));
ctx->state_function = csm_complete_connect;
if (!(ret = mysql_socket_getsockopt(vio->mysql_socket, SOL_SOCKET, SO_ERROR,
optval, &optlen))) {
#ifdef _WIN32
WSASetLastError(error);
#else
errno = error;
#endif
if (error != 0) {
DBUG_PRINT("error",
("Got error %d on connect to '%s'", error, ctx->host));
set_mysql_extended_error(
ctx->mysql, CR_CONN_HOST_ERROR, unknown_sqlstate,
ER_CLIENT(CR_CONN_HOST_ERROR), ctx->host, error);
return STATE_MACHINE_FAILED;
}
}
}
return STATE_MACHINE_CONTINUE;
}

/**
Complete the connection itself, setting options on the now-connected socket.
*/
Expand Down
33 changes: 24 additions & 9 deletions testclients/mysql_client_test.cc
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (c) 2002, 2019, Oracle and/or its affiliates. All rights reserved.
/* Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License, version 2.0,
Expand Down Expand Up @@ -20181,11 +20181,14 @@ static void test_wl11381() {
myerror("mysql_client_init() failed");
exit(1);
}
status = mysql_real_connect_nonblocking(
mysql_local, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS);
do {
status = mysql_real_connect_nonblocking(
mysql_local, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS);
} while (status == NET_ASYNC_NOT_READY);
if (status == NET_ASYNC_ERROR) {
fprintf(stdout, "\n mysql_real_connect_nonblocking() failed");
fprintf(stdout, "\n mysql_real_connect_nonblocking() failed. Error: [%s]",
mysql_error(mysql_local));
exit(1);
} else {
fprintf(stdout, "\n asynchronous connection estalished");
Expand Down Expand Up @@ -20377,13 +20380,25 @@ static void test_wl11381_qa() {
exit(1);
}

mysql_con1_status = (mysql_real_connect_nonblocking(
mysql_con1_status = mysql_real_connect_nonblocking(
mysql_con1, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS));
opt_unix_socket, CLIENT_MULTI_STATEMENTS);

mysql_con2_status = (mysql_real_connect_nonblocking(
mysql_con2_status = mysql_real_connect_nonblocking(
mysql_con2, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS));
opt_unix_socket, CLIENT_MULTI_STATEMENTS);

while (mysql_con1_status == NET_ASYNC_NOT_READY) {
mysql_con1_status = mysql_real_connect_nonblocking(
mysql_con1, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS);
}

while (mysql_con2_status == NET_ASYNC_NOT_READY) {
mysql_con2_status = mysql_real_connect_nonblocking(
mysql_con2, opt_host, opt_user, opt_password, current_db, opt_port,
opt_unix_socket, CLIENT_MULTI_STATEMENTS);
}

if (mysql_con1_status == NET_ASYNC_ERROR) {
fprintf(stdout, "\n mysql_real_connect_nonblocking() failed");
Expand Down
11 changes: 9 additions & 2 deletions vio/viosocket.cc
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License, version 2.0,
Expand Down Expand Up @@ -1028,13 +1028,16 @@ int vio_io_wait(Vio *vio, enum enum_vio_io_event event, int timeout) {
@param nonblocking flag to represent if socket is blocking or nonblocking
@param timeout Interval (in milliseconds) to wait until a
connection is established.
@param [out] connect_done Indication if connect actually completed or not.
If set to true this means there's no need to wait for
connect to complete anymore.
@retval false A connection was successfully established.
@retval true A fatal error. See socket_errno.
*/

bool vio_socket_connect(Vio *vio, struct sockaddr *addr, socklen_t len,
bool nonblocking, int timeout) {
bool nonblocking, int timeout, bool *connect_done) {
int ret, wait;
int retry_count = 0;
DBUG_TRACE;
Expand All @@ -1052,6 +1055,8 @@ bool vio_socket_connect(Vio *vio, struct sockaddr *addr, socklen_t len,
} while (ret < 0 && vio_should_retry(vio) &&
(retry_count++ < vio->retry_count));

if (connect_done) *connect_done = (ret == 0);

#ifdef _WIN32
wait = (ret == SOCKET_ERROR) && (WSAGetLastError() == WSAEINPROGRESS ||
WSAGetLastError() == WSAEWOULDBLOCK);
Expand Down Expand Up @@ -1087,6 +1092,7 @@ bool vio_socket_connect(Vio *vio, struct sockaddr *addr, socklen_t len,
it was really a success. Otherwise we might prevent the caller
from trying another address to connect to.
*/
if (connect_done) *connect_done = true;
if (!(ret = mysql_socket_getsockopt(vio->mysql_socket, SOL_SOCKET, SO_ERROR,
optval, &optlen))) {
#ifdef _WIN32
Expand All @@ -1104,6 +1110,7 @@ bool vio_socket_connect(Vio *vio, struct sockaddr *addr, socklen_t len,
}

if (nonblocking && wait) {
if (connect_done) *connect_done = false;
return false;
} else {
return (ret != 0);
Expand Down

0 comments on commit 1923bb7

Please sign in to comment.