Commit 8ce93bbd authored by Michael Widenius's avatar Michael Widenius

Fixed bug that MAX_USER_CONNECTIONS was not working properly in all situations...

Fixed bug that MAX_USER_CONNECTIONS was not working properly in all situations (which could cause aborted connects)
thd->user_connect is now handled in thd->clenup() which will ensure that it works in all context (including slaves).
I added also some DBUG_ASSERT() to ensure that things are working correctly.


sql/sql_acl.cc:
  Reset thd->user_connect on failed check_for_max_user_connections() to ensure we don't decrement value twice.
  Removed not needed call to decrease_user_connections() as thd->cleanup() will now do it.
sql/sql_class.cc:
  Call decrease_user_connections() in thd->cleanup()
sql/sql_connect.cc:
  Ensure we don't allocate thd->user_connect twice.
  Simplify check_for_max_user_connections().
sql/sql_parse.cc:
  Ensure that thd->user_connect is handled properly in for 'change_user' command.
parent 13af3982
...@@ -8167,6 +8167,8 @@ bool acl_authenticate(THD *thd, uint connect_errors, ...@@ -8167,6 +8167,8 @@ bool acl_authenticate(THD *thd, uint connect_errors,
max_user_connections) && max_user_connections) &&
check_for_max_user_connections(thd, thd->user_connect)) check_for_max_user_connections(thd, thd->user_connect))
{ {
/* Ensure we don't decrement thd->user_connections->connections twice */
thd->user_connect= 0;
status_var_increment(denied_connections); status_var_increment(denied_connections);
DBUG_RETURN(1); // The error is set in check_for_max_user_connections() DBUG_RETURN(1); // The error is set in check_for_max_user_connections()
} }
...@@ -8207,12 +8209,7 @@ bool acl_authenticate(THD *thd, uint connect_errors, ...@@ -8207,12 +8209,7 @@ bool acl_authenticate(THD *thd, uint connect_errors,
if (mysql_change_db(thd, &mpvio.db, FALSE)) if (mysql_change_db(thd, &mpvio.db, FALSE))
{ {
/* mysql_change_db() has pushed the error message. */ /* mysql_change_db() has pushed the error message. */
if (thd->user_connect)
{
status_var_increment(thd->status_var.access_denied_errors); status_var_increment(thd->status_var.access_denied_errors);
decrease_user_connections(thd->user_connect);
thd->user_connect= 0;
}
DBUG_RETURN(1); DBUG_RETURN(1);
} }
} }
......
...@@ -1080,6 +1080,11 @@ void THD::cleanup(void) ...@@ -1080,6 +1080,11 @@ void THD::cleanup(void)
lock=locked_tables; locked_tables=0; lock=locked_tables; locked_tables=0;
close_thread_tables(this); close_thread_tables(this);
} }
if (user_connect)
{
decrease_user_connections(user_connect);
user_connect= 0; // Safety
}
wt_thd_destroy(&transaction.wt); wt_thd_destroy(&transaction.wt);
#if defined(ENABLED_DEBUG_SYNC) #if defined(ENABLED_DEBUG_SYNC)
......
...@@ -49,6 +49,7 @@ int get_or_create_user_conn(THD *thd, const char *user, ...@@ -49,6 +49,7 @@ int get_or_create_user_conn(THD *thd, const char *user,
DBUG_ASSERT(user != 0); DBUG_ASSERT(user != 0);
DBUG_ASSERT(host != 0); DBUG_ASSERT(host != 0);
DBUG_ASSERT(thd->user_connect == 0);
user_len= strlen(user); user_len= strlen(user);
temp_len= (strmov(strmov(temp_user, user)+1, host) - temp_user)+1; temp_len= (strmov(strmov(temp_user, user)+1, host) - temp_user)+1;
...@@ -108,7 +109,7 @@ end: ...@@ -108,7 +109,7 @@ end:
int check_for_max_user_connections(THD *thd, USER_CONN *uc) int check_for_max_user_connections(THD *thd, USER_CONN *uc)
{ {
int error=0; int error= 1;
DBUG_ENTER("check_for_max_user_connections"); DBUG_ENTER("check_for_max_user_connections");
(void) pthread_mutex_lock(&LOCK_user_conn); (void) pthread_mutex_lock(&LOCK_user_conn);
...@@ -116,7 +117,6 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc) ...@@ -116,7 +117,6 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc)
max_user_connections < (uint) uc->connections) max_user_connections < (uint) uc->connections)
{ {
my_error(ER_TOO_MANY_USER_CONNECTIONS, MYF(0), uc->user); my_error(ER_TOO_MANY_USER_CONNECTIONS, MYF(0), uc->user);
error=1;
goto end; goto end;
} }
time_out_user_resource_limits(thd, uc); time_out_user_resource_limits(thd, uc);
...@@ -126,7 +126,6 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc) ...@@ -126,7 +126,6 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc)
my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user,
"max_user_connections", "max_user_connections",
(long) uc->user_resources.user_conn); (long) uc->user_resources.user_conn);
error= 1;
goto end; goto end;
} }
if (uc->user_resources.conn_per_hour && if (uc->user_resources.conn_per_hour &&
...@@ -135,10 +134,10 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc) ...@@ -135,10 +134,10 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc)
my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user,
"max_connections_per_hour", "max_connections_per_hour",
(long) uc->user_resources.conn_per_hour); (long) uc->user_resources.conn_per_hour);
error=1;
goto end; goto end;
} }
uc->conn_per_hour++; uc->conn_per_hour++;
error= 0;
end: end:
if (error) if (error)
...@@ -1027,8 +1026,17 @@ void end_connection(THD *thd) ...@@ -1027,8 +1026,17 @@ void end_connection(THD *thd)
{ {
NET *net= &thd->net; NET *net= &thd->net;
plugin_thdvar_cleanup(thd); plugin_thdvar_cleanup(thd);
if (thd->user_connect) if (thd->user_connect)
{
/*
We decrease this variable early to make it easy to log again quickly.
This code is not critical as we will in any case do this test
again in thd->cleanup()
*/
decrease_user_connections(thd->user_connect); decrease_user_connections(thd->user_connect);
thd->user_connect= 0;
}
if (thd->killed || (net->error && net->vio != 0)) if (thd->killed || (net->error && net->vio != 0))
{ {
......
...@@ -1138,12 +1138,15 @@ bool dispatch_command(enum enum_server_command command, THD *thd, ...@@ -1138,12 +1138,15 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
/* Ensure we don't free security_ctx->user in case we have to revert */ /* Ensure we don't free security_ctx->user in case we have to revert */
thd->security_ctx->user= 0; thd->security_ctx->user= 0;
thd->user_connect= 0;
if (acl_authenticate(thd, 0, packet_length)) if (acl_authenticate(thd, 0, packet_length))
{ {
/* Free user if allocated by acl_authenticate */ /* Free user if allocated by acl_authenticate */
x_free(thd->security_ctx->user); x_free(thd->security_ctx->user);
*thd->security_ctx= save_security_ctx; *thd->security_ctx= save_security_ctx;
if (thd->user_connect)
decrease_user_connections(thd->user_connect);
thd->user_connect= save_user_connect; thd->user_connect= save_user_connect;
thd->reset_db(save_db, save_db_length); thd->reset_db(save_db, save_db_length);
thd->variables.character_set_client= save_character_set_client; thd->variables.character_set_client= save_character_set_client;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment