summaryrefslogtreecommitdiff
path: root/sql
diff options
context:
space:
mode:
authorThayumanavar <thayumanavar.x.sachithanantha@oracle.com>2014-01-13 12:04:16 +0530
committerThayumanavar <thayumanavar.x.sachithanantha@oracle.com>2014-01-13 12:04:16 +0530
commit819eb3e01a8a11835a0e3cb5a51862677d150b77 (patch)
tree7963555fa99e0cdafed0476c71cdbc81b6ac48f9 /sql
parentff6b117c364698c50b177e62475a8bebf92d4980 (diff)
downloadmariadb-git-819eb3e01a8a11835a0e3cb5a51862677d150b77.tar.gz
BUG#18054998 - BACKPORT FIX FOR BUG#11765785 to 5.5
This is a backport of the patch of bug#11765785. Commit message by Prabakaran Thirumalai from bug#11765785 is reproduced below: Description: ------------ Global Query ID (global_query_id ) is not incremented for PING and statistics command. These two query types are filtered before incrementing the global query id. This causes race condition and results in duplicate query id for different queries originating from different connections. Analysis: --------- sqlparse.cc::dispath_command() is the only place in code which sets thd->query_ id to global_query_id and then increments it based on the query type. In all other places it is incremented first and then assigned to thd->query_id. This is done such that global_query_id is not incremented for PING and statistics commands in dispatch_command() function. Fix: ---- As per suggestion from Serg, "There is no reason to skip query_id for the PING and STATISTICS command.", removing the check which filters PING and statistics commands. Instead of using get_query_id() and next_query_id() which can still cause race condition if context switch happens soon after executing get_query_id(), changing the code to use next_query_id() instead of get_query_id() as it is done in other parts of code which deals with global_query_id. Removed get_query_id() function and forced next_query_id() caller to use the return value by specifying warn_unused_result attribute.
Diffstat (limited to 'sql')
-rw-r--r--sql/mysqld.h12
-rw-r--r--sql/sql_class.h4
-rw-r--r--sql/sql_parse.cc8
3 files changed, 7 insertions, 17 deletions
diff --git a/sql/mysqld.h b/sql/mysqld.h
index e581b413e0d..f500a0abf3f 100644
--- a/sql/mysqld.h
+++ b/sql/mysqld.h
@@ -438,7 +438,7 @@ extern my_atomic_rwlock_t global_query_id_lock;
void unireg_end(void) __attribute__((noreturn));
/* increment query_id and return it. */
-inline query_id_t next_query_id()
+inline __attribute__((warn_unused_result)) query_id_t next_query_id()
{
query_id_t id;
my_atomic_rwlock_wrlock(&global_query_id_lock);
@@ -447,16 +447,6 @@ inline query_id_t next_query_id()
return (id+1);
}
-inline query_id_t get_query_id()
-{
- query_id_t id;
- my_atomic_rwlock_wrlock(&global_query_id_lock);
- id= my_atomic_load64(&global_query_id);
- my_atomic_rwlock_wrunlock(&global_query_id_lock);
- return id;
-}
-
-
/*
TODO: Replace this with an inline function.
*/
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 7211eca4939..58328713e77 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -3667,7 +3667,9 @@ public:
/**
Skip the increase of the global query id counter. Commonly set for
commands that are stateless (won't cause any change on the server
- internal states).
+ internal states). This is made obsolete as query id is incremented for
+ ping and statistics commands as well because of race condition
+ (Bug#58785).
*/
#define CF_SKIP_QUERY_ID (1U << 0)
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 6e2b422bd44..fe9f564672a 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -237,8 +237,8 @@ void init_update_queries(void)
/* Initialize the server command flags array. */
memset(server_command_flags, 0, sizeof(server_command_flags));
- server_command_flags[COM_STATISTICS]= CF_SKIP_QUERY_ID | CF_SKIP_QUESTIONS;
- server_command_flags[COM_PING]= CF_SKIP_QUERY_ID | CF_SKIP_QUESTIONS;
+ server_command_flags[COM_STATISTICS]= CF_SKIP_QUESTIONS;
+ server_command_flags[COM_PING]= CF_SKIP_QUESTIONS;
server_command_flags[COM_STMT_PREPARE]= CF_SKIP_QUESTIONS;
server_command_flags[COM_STMT_CLOSE]= CF_SKIP_QUESTIONS;
server_command_flags[COM_STMT_RESET]= CF_SKIP_QUESTIONS;
@@ -901,9 +901,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
thd->security_ctx->master_access|= SHUTDOWN_ACL;
command= COM_SHUTDOWN;
}
- thd->set_query_id(get_query_id());
- if (!(server_command_flags[command] & CF_SKIP_QUERY_ID))
- next_query_id();
+ thd->set_query_id(next_query_id());
inc_thread_running();
if (!(server_command_flags[command] & CF_SKIP_QUESTIONS))