Skip to content

Commit 589c7b4

Browse files
committed
BUG# 34574013 - backport 33723597 to 5.7
Backporting the bug 33723597 to 5.7 A connect packet can read in chunks. But the connect_timeout applies to reading the individual chunks. And not to the full packet being read. This can lead to very long reads in the client is taking its time to write the full packet. Fixed by also enforcing connect_timeout on the full packet read and not only to the individual chunks. Additional Changes: In previous code base, it was only the socket connection types which had server extension (ext) along with them, other connection types such as named pipe, shared memory etc. didn't have them. In order to fetch the timeout_on_full_packet from the NET_SERVER for named pipe, and shared memory connections, we need to have this extension available to all connection types, not just to socket connections. Moved the "init_net_server_extension" from socket_connection.cc to a common header and source file. Added the function calls to named_pipe_connection.cc as well as shared_memory_connection.cc, and modified socket_connection.cc accordingly. Change-Id: Ibbaff8e26445b04d04f4e812be561eefe150157f
1 parent 1c930fd commit 589c7b4

11 files changed

+197
-94
lines changed

include/mysql_com_server.h

+8
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ struct st_net_server
4141
before_header_callback_fn m_before_header;
4242
after_header_callback_fn m_after_header;
4343
void *m_user_data;
44+
my_bool timeout_on_full_packet;
45+
46+
st_net_server() {
47+
m_before_header = NULL;
48+
m_after_header = NULL;
49+
m_user_data = NULL;
50+
timeout_on_full_packet = FALSE;
51+
}
4452
};
4553

4654
typedef struct st_net_server NET_SERVER;

sql/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ SET(SQL_SOURCE
281281
conn_handler/connection_handler_per_thread.cc
282282
conn_handler/connection_handler_one_thread.cc
283283
conn_handler/socket_connection.cc
284+
conn_handler/init_net_server_extension.cc
284285
des_key_file.cc
285286
event_data_objects.cc
286287
event_db_repository.cc
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/*
2+
Copyright (c) 2013, 2022, Oracle and/or its affiliates.
3+
4+
This program is free software; you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License, version 2.0,
6+
as published by the Free Software Foundation.
7+
8+
This program is also distributed with certain software (including
9+
but not limited to OpenSSL) that is licensed under separate terms,
10+
as designated in a particular file or component or in included license
11+
documentation. The authors of MySQL hereby grant you an additional
12+
permission to link the program and your derivative works with the
13+
separately licensed software that they have included with MySQL.
14+
15+
This program is distributed in the hope that it will be useful,
16+
but WITHOUT ANY WARRANTY; without even the implied warranty of
17+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18+
GNU General Public License, version 2.0, for more details.
19+
20+
You should have received a copy of the GNU General Public License
21+
along with this program; if not, write to the Free Software
22+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
23+
*/
24+
25+
#include "init_net_server_extension.h"
26+
27+
#include "violite.h" // Vio
28+
#include "channel_info.h" // Channel_info
29+
#include "connection_handler_manager.h" // Connection_handler_manager
30+
#include "mysqld.h" // key_socket_tcpip
31+
#include "log.h" // sql_print_error
32+
#include "sql_class.h" // THD
33+
34+
#include <pfs_idle_provider.h>
35+
#include <mysql/psi/mysql_idle.h>
36+
37+
#ifdef HAVE_PSI_STATEMENT_INTERFACE
38+
extern PSI_statement_info stmt_info_new_packet;
39+
#endif
40+
41+
static void net_before_header_psi(struct st_net *net, void *user_data,
42+
size_t /* unused: count */) {
43+
THD *thd;
44+
thd = static_cast<THD *>(user_data);
45+
assert(thd != NULL);
46+
47+
if (thd->m_server_idle) {
48+
/*
49+
The server is IDLE, waiting for the next command.
50+
Technically, it is a wait on a socket, which may take a long time,
51+
because the call is blocking.
52+
Disable the socket instrumentation, to avoid recording a SOCKET event.
53+
Instead, start explicitly an IDLE event.
54+
*/
55+
MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_IDLE);
56+
MYSQL_START_IDLE_WAIT(thd->m_idle_psi, &thd->m_idle_state);
57+
}
58+
}
59+
60+
static void net_after_header_psi(struct st_net *net, void *user_data,
61+
size_t /* unused: count */, my_bool rc) {
62+
THD *thd;
63+
thd = static_cast<THD *>(user_data);
64+
assert(thd != NULL);
65+
66+
if (thd->m_server_idle) {
67+
/*
68+
The server just got data for a network packet header,
69+
from the network layer.
70+
The IDLE event is now complete, since we now have a message to process.
71+
We need to:
72+
- start a new STATEMENT event
73+
- start a new STAGE event, within this statement,
74+
- start recording SOCKET WAITS events, within this stage.
75+
The proper order is critical to get events numbered correctly,
76+
and nested in the proper parent.
77+
*/
78+
MYSQL_END_IDLE_WAIT(thd->m_idle_psi);
79+
80+
if (!rc) {
81+
assert(thd->m_statement_psi == NULL);
82+
thd->m_statement_psi = MYSQL_START_STATEMENT(
83+
&thd->m_statement_state, stmt_info_new_packet.m_key, thd->db().str,
84+
thd->db().length, thd->charset(), NULL);
85+
86+
/*
87+
Starts a new stage in performance schema, if compiled in and enabled.
88+
Also sets THD::proc_info (used by SHOW PROCESSLIST, column STATE)
89+
*/
90+
THD_STAGE_INFO(thd, stage_starting);
91+
}
92+
93+
/*
94+
TODO: consider recording a SOCKET event for the bytes just read,
95+
by also passing count here.
96+
*/
97+
MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_ACTIVE);
98+
thd->m_server_idle = false;
99+
}
100+
}
101+
102+
void init_net_server_extension(THD *thd) {
103+
/* Start with a clean state for connection events. */
104+
thd->m_idle_psi = NULL;
105+
thd->m_statement_psi = NULL;
106+
thd->m_server_idle = false;
107+
108+
/* Hook up the NET_SERVER callback in the net layer. */
109+
thd->m_net_server_extension.m_user_data = thd;
110+
thd->m_net_server_extension.m_before_header = net_before_header_psi;
111+
thd->m_net_server_extension.m_after_header = net_after_header_psi;
112+
thd->m_net_server_extension.timeout_on_full_packet = FALSE;
113+
114+
/* Activate this private extension for the mysqld server. */
115+
thd->get_protocol_classic()->get_net()->extension =
116+
&thd->m_net_server_extension;
117+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
Copyright (c) 2013, 2022, Oracle and/or its affiliates.
3+
4+
This program is free software; you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License, version 2.0,
6+
as published by the Free Software Foundation.
7+
8+
This program is also distributed with certain software (including
9+
but not limited to OpenSSL) that is licensed under separate terms,
10+
as designated in a particular file or component or in included license
11+
documentation. The authors of MySQL hereby grant you an additional
12+
permission to link the program and your derivative works with the
13+
separately licensed software that they have included with MySQL.
14+
15+
This program is distributed in the hope that it will be useful,
16+
but WITHOUT ANY WARRANTY; without even the implied warranty of
17+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18+
GNU General Public License, version 2.0, for more details.
19+
20+
You should have received a copy of the GNU General Public License
21+
along with this program; if not, write to the Free Software
22+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
23+
*/
24+
25+
#ifndef INIT_NET_SERVER_EXTENSION_INCLUDED
26+
#define INIT_NET_SERVER_EXTENSION_INCLUDED
27+
28+
class THD;
29+
30+
void init_net_server_extension(THD *thd);
31+
32+
#endif

sql/conn_handler/named_pipe_connection.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
2323
*/
2424

25+
#include "init_net_server_extension.h"
2526
#include "named_pipe_connection.h"
2627

2728
#include <AclAPI.h> // Windows access control API
@@ -66,8 +67,10 @@ class Channel_info_named_pipe : public Channel_info
6667
{
6768
THD* thd= Channel_info::create_thd();
6869

69-
if (thd != NULL)
70-
thd->security_context()->set_host_ptr(my_localhost, strlen(my_localhost));
70+
if (thd != NULL) {
71+
init_net_server_extension(thd);
72+
thd->security_context()->set_host_ptr(my_localhost, strlen(my_localhost));
73+
}
7174
return thd;
7275
}
7376

sql/conn_handler/shared_memory_connection.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
2323
*/
2424

25+
#include "init_net_server_extension.h"
2526
#include "shared_memory_connection.h"
2627

2728
#include "violite.h" // Vio
@@ -93,8 +94,10 @@ class Channel_info_shared_mem : public Channel_info
9394
{
9495
THD* thd= Channel_info::create_thd();
9596

96-
if (thd != NULL)
97-
thd->security_context()->set_host_ptr(my_localhost, strlen(my_localhost));
97+
if (thd != NULL) {
98+
init_net_server_extension(thd);
99+
thd->security_context()->set_host_ptr(my_localhost, strlen(my_localhost));
100+
}
98101
return thd;
99102
}
100103

sql/conn_handler/socket_connection.cc

+1-86
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "violite.h" // Vio
2828
#include "channel_info.h" // Channel_info
2929
#include "connection_handler_manager.h" // Connection_handler_manager
30+
#include "init_net_server_extension.h"
3031
#include "mysqld.h" // key_socket_tcpip
3132
#include "log.h" // sql_print_error
3233
#include "sql_class.h" // THD
@@ -56,92 +57,6 @@ ulong Mysqld_socket_listener::connection_errors_select= 0;
5657
ulong Mysqld_socket_listener::connection_errors_accept= 0;
5758
ulong Mysqld_socket_listener::connection_errors_tcpwrap= 0;
5859

59-
60-
void net_before_header_psi(struct st_net *net, void *user_data, size_t /* unused: count */)
61-
{
62-
THD *thd;
63-
thd= static_cast<THD*> (user_data);
64-
assert(thd != NULL);
65-
66-
if (thd->m_server_idle)
67-
{
68-
/*
69-
The server is IDLE, waiting for the next command.
70-
Technically, it is a wait on a socket, which may take a long time,
71-
because the call is blocking.
72-
Disable the socket instrumentation, to avoid recording a SOCKET event.
73-
Instead, start explicitly an IDLE event.
74-
*/
75-
MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_IDLE);
76-
MYSQL_START_IDLE_WAIT(thd->m_idle_psi, &thd->m_idle_state);
77-
}
78-
}
79-
80-
void net_after_header_psi(struct st_net *net, void *user_data, size_t /* unused: count */, my_bool rc)
81-
{
82-
THD *thd;
83-
thd= static_cast<THD*> (user_data);
84-
assert(thd != NULL);
85-
86-
if (thd->m_server_idle)
87-
{
88-
/*
89-
The server just got data for a network packet header,
90-
from the network layer.
91-
The IDLE event is now complete, since we now have a message to process.
92-
We need to:
93-
- start a new STATEMENT event
94-
- start a new STAGE event, within this statement,
95-
- start recording SOCKET WAITS events, within this stage.
96-
The proper order is critical to get events numbered correctly,
97-
and nested in the proper parent.
98-
*/
99-
MYSQL_END_IDLE_WAIT(thd->m_idle_psi);
100-
101-
if (! rc)
102-
{
103-
assert(thd->m_statement_psi == NULL);
104-
thd->m_statement_psi= MYSQL_START_STATEMENT(&thd->m_statement_state,
105-
stmt_info_new_packet.m_key,
106-
thd->db().str,
107-
thd->db().length,
108-
thd->charset(), NULL);
109-
110-
/*
111-
Starts a new stage in performance schema, if compiled in and enabled.
112-
Also sets THD::proc_info (used by SHOW PROCESSLIST, column STATE)
113-
*/
114-
THD_STAGE_INFO(thd, stage_starting);
115-
}
116-
117-
/*
118-
TODO: consider recording a SOCKET event for the bytes just read,
119-
by also passing count here.
120-
*/
121-
MYSQL_SOCKET_SET_STATE(net->vio->mysql_socket, PSI_SOCKET_STATE_ACTIVE);
122-
thd->m_server_idle= false;
123-
}
124-
}
125-
126-
127-
static void init_net_server_extension(THD *thd)
128-
{
129-
/* Start with a clean state for connection events. */
130-
thd->m_idle_psi= NULL;
131-
thd->m_statement_psi= NULL;
132-
thd->m_server_idle= false;
133-
134-
/* Hook up the NET_SERVER callback in the net layer. */
135-
thd->m_net_server_extension.m_user_data= thd;
136-
thd->m_net_server_extension.m_before_header= net_before_header_psi;
137-
thd->m_net_server_extension.m_after_header= net_after_header_psi;
138-
139-
/* Activate this private extension for the mysqld server. */
140-
thd->get_protocol_classic()->get_net()->extension=
141-
&thd->m_net_server_extension;
142-
}
143-
144-
14560
///////////////////////////////////////////////////////////////////////////
14661
// Channel_info_local_socket implementation
14762
///////////////////////////////////////////////////////////////////////////

sql/net_serv.cc

+20-1
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,16 @@ static my_bool net_read_raw_loop(NET *net, size_t count)
698698
bool eof= false;
699699
unsigned int retry_count= 0;
700700
uchar *buf= net->buff + net->where_b;
701+
my_bool timeout_on_full_packet = FALSE;
702+
my_bool is_packet_timeout = FALSE;
703+
704+
#ifdef MYSQL_SERVER
705+
NET_SERVER* server_ext = static_cast<NET_SERVER*>(net->extension);
706+
if (server_ext) timeout_on_full_packet = server_ext->timeout_on_full_packet;
707+
#endif
708+
709+
time_t start_time = 0;
710+
if (timeout_on_full_packet) start_time = time(&start_time);
701711

702712
while (count)
703713
{
@@ -724,6 +734,15 @@ static my_bool net_read_raw_loop(NET *net, size_t count)
724734
#ifdef MYSQL_SERVER
725735
thd_increment_bytes_received(recvcnt);
726736
#endif
737+
738+
if (timeout_on_full_packet) {
739+
time_t current_time = time(&current_time);
740+
if (static_cast<unsigned int>(current_time - start_time) >
741+
net->read_timeout) {
742+
is_packet_timeout = TRUE;
743+
break;
744+
}
745+
}
727746
}
728747

729748
/* On failure, propagate the error code. */
@@ -733,7 +752,7 @@ static my_bool net_read_raw_loop(NET *net, size_t count)
733752
net->error= 2;
734753

735754
/* Interrupted by a timeout? */
736-
if (!eof && vio_was_timeout(net->vio))
755+
if (!eof && (vio_was_timeout(net->vio) || is_packet_timeout))
737756
net->last_errno= ER_NET_READ_INTERRUPTED;
738757
else
739758
net->last_errno= ER_NET_READ_ERROR;

sql/protocol_classic.cc

+5-1
Original file line numberDiff line numberDiff line change
@@ -695,9 +695,13 @@ bool Protocol_classic::send_error(uint sql_errno, const char *err_msg,
695695
}
696696

697697

698-
void Protocol_classic::set_read_timeout(ulong read_timeout)
698+
void Protocol_classic::set_read_timeout(ulong read_timeout,
699+
my_bool on_full_packet)
699700
{
700701
my_net_set_read_timeout(&m_thd->net, read_timeout);
702+
NET_SERVER* ext = static_cast<NET_SERVER*>(m_thd->net.extension);
703+
assert(ext);
704+
ext->timeout_on_full_packet = on_full_packet;
701705
}
702706

703707

sql/protocol_classic.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ class Protocol_classic : public Protocol
206206
/* Return raw packet buffer */
207207
uchar *get_raw_packet() { return raw_packet; }
208208
/* Set read timeout */
209-
virtual void set_read_timeout(ulong read_timeout);
209+
virtual void set_read_timeout(ulong read_timeout,
210+
my_bool on_full_packet = FALSE);
210211
/* Set write timeout */
211212
virtual void set_write_timeout(ulong write_timeout);
212213
};

sql/sql_connect.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ static bool login_connection(THD *thd)
753753
thd->thread_id()));
754754

755755
/* Use "connect_timeout" value during connection phase */
756-
thd->get_protocol_classic()->set_read_timeout(connect_timeout);
756+
thd->get_protocol_classic()->set_read_timeout(connect_timeout, TRUE);
757757
thd->get_protocol_classic()->set_write_timeout(connect_timeout);
758758

759759
error= check_connection(thd);

0 commit comments

Comments
 (0)