Skip to content

Commit 5f31dbd

Browse files
author
Gleb Shchepa
committed
Bug #38883 (reopened): thd_security_context is not thread safe, crashes?
The bug 38816 changed the lock that protects THD::query from LOCK_thread_count to LOCK_thd_data, but didn't update the associated InnoDB functions. 1. The innobase_mysql_prepare_print_arbitrary_thd and the innobase_mysql_end_print_arbitrary_thd InnoDB functions have been removed, since now we have a per-thread mutex: now we don't need to wrap several inter-thread access tries to THD::query with a single global LOCK_thread_count lock, so we can simplify the code. 2. The innobase_mysql_print_thd function has been modified to lock LOCK_thd_data in direct way.
1 parent e35a4db commit 5f31dbd

File tree

4 files changed

+10
-70
lines changed

4 files changed

+10
-70
lines changed

innobase/include/trx0trx.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,7 @@ trx_commit_step(
348348

349349
/**************************************************************************
350350
Prints info about a transaction to the given file. The caller must own the
351-
kernel mutex and must have called
352-
innobase_mysql_prepare_print_arbitrary_thd(), unless he knows that MySQL
353-
or InnoDB cannot meanwhile change the info printed here. */
351+
kernel mutex. */
354352

355353
void
356354
trx_print(

innobase/lock/lock0lock.c

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,6 @@ Created 5/7/1996 Heikki Tuuri
1818
#include "trx0sys.h"
1919

2020

21-
/* 2 function prototypes copied from ha_innodb.cc: */
22-
23-
/*****************************************************************
24-
If you want to print a thd that is not associated with the current thread,
25-
you must call this function before reserving the InnoDB kernel_mutex, to
26-
protect MySQL from setting thd->query NULL. If you print a thd of the current
27-
thread, we know that MySQL cannot modify thd->query, and it is not necessary
28-
to call this. Call innobase_mysql_end_print_arbitrary_thd() after you release
29-
the kernel_mutex.
30-
NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
31-
function! */
32-
33-
void
34-
innobase_mysql_prepare_print_arbitrary_thd(void);
35-
/*============================================*/
36-
37-
/*****************************************************************
38-
Relases the mutex reserved by innobase_mysql_prepare_print_arbitrary_thd().
39-
NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
40-
function! */
41-
42-
void
43-
innobase_mysql_end_print_arbitrary_thd(void);
44-
/*========================================*/
45-
4621
/* Restricts the length of search we will do in the waits-for
4722
graph of transactions */
4823
#define LOCK_MAX_N_STEPS_IN_DEADLOCK_CHECK 1000000
@@ -4231,11 +4206,6 @@ lock_print_info_summary(
42314206
/*====================*/
42324207
FILE* file) /* in: file where to print */
42334208
{
4234-
/* We must protect the MySQL thd->query field with a MySQL mutex, and
4235-
because the MySQL mutex must be reserved before the kernel_mutex of
4236-
InnoDB, we call innobase_mysql_prepare_print_arbitrary_thd() here. */
4237-
4238-
innobase_mysql_prepare_print_arbitrary_thd();
42394209
lock_mutex_enter_kernel();
42404210

42414211
if (lock_deadlock_found) {
@@ -4322,7 +4292,6 @@ lock_print_info_all_transactions(
43224292

43234293
if (trx == NULL) {
43244294
lock_mutex_exit_kernel();
4325-
innobase_mysql_end_print_arbitrary_thd();
43264295

43274296
ut_ad(lock_validate());
43284297

@@ -4387,7 +4356,6 @@ lock_print_info_all_transactions(
43874356

43884357
if (load_page_first) {
43894358
lock_mutex_exit_kernel();
4390-
innobase_mysql_end_print_arbitrary_thd();
43914359

43924360
mtr_start(&mtr);
43934361

@@ -4397,7 +4365,6 @@ lock_print_info_all_transactions(
43974365

43984366
load_page_first = FALSE;
43994367

4400-
innobase_mysql_prepare_print_arbitrary_thd();
44014368
lock_mutex_enter_kernel();
44024369

44034370
goto loop;

innobase/trx/trx0trx.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,9 +1701,7 @@ trx_mark_sql_stat_end(
17011701

17021702
/**************************************************************************
17031703
Prints info about a transaction to the given file. The caller must own the
1704-
kernel mutex and must have called
1705-
innobase_mysql_prepare_print_arbitrary_thd(), unless he knows that MySQL
1706-
or InnoDB cannot meanwhile change the info printed here. */
1704+
kernel mutex. */
17071705

17081706
void
17091707
trx_print(

sql/ha_innodb.cc

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -554,35 +554,6 @@ convert_error_code_to_mysql(
554554
}
555555
}
556556

557-
/*****************************************************************
558-
If you want to print a thd that is not associated with the current thread,
559-
you must call this function before reserving the InnoDB kernel_mutex, to
560-
protect MySQL from setting thd->query NULL. If you print a thd of the current
561-
thread, we know that MySQL cannot modify thd->query, and it is not necessary
562-
to call this. Call innobase_mysql_end_print_arbitrary_thd() after you release
563-
the kernel_mutex.
564-
NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
565-
function! */
566-
extern "C"
567-
void
568-
innobase_mysql_prepare_print_arbitrary_thd(void)
569-
/*============================================*/
570-
{
571-
VOID(pthread_mutex_lock(&LOCK_thread_count));
572-
}
573-
574-
/*****************************************************************
575-
Releases the mutex reserved by innobase_mysql_prepare_print_arbitrary_thd().
576-
NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
577-
function! */
578-
extern "C"
579-
void
580-
innobase_mysql_end_print_arbitrary_thd(void)
581-
/*========================================*/
582-
{
583-
VOID(pthread_mutex_unlock(&LOCK_thread_count));
584-
}
585-
586557
/*****************************************************************
587558
Prints info of a THD object (== user session thread) to the given file.
588559
NOTE that /mysql/innobase/trx/trx0trx.c must contain the prototype for
@@ -596,11 +567,11 @@ innobase_mysql_print_thd(
596567
uint max_query_len) /* in: max query length to print, or 0 to
597568
use the default max length */
598569
{
599-
const THD* thd;
570+
THD* thd;
600571
const Security_context *sctx;
601572
const char* s;
602573

603-
thd = (const THD*) input_thd;
574+
thd = (THD*) input_thd;
604575
/* We probably want to have original user as part of debug output. */
605576
sctx = &thd->main_security_ctx;
606577

@@ -627,6 +598,10 @@ innobase_mysql_print_thd(
627598
fputs(s, f);
628599
}
629600

601+
/* We have to quarantine an access to thd->query and
602+
thd->query_length with thd->LOCK_thd_data mutex. */
603+
VOID(pthread_mutex_lock(&thd->LOCK_thd_data));
604+
630605
if ((s = thd->query)) {
631606
/* 3100 is chosen because currently 3000 is the maximum
632607
max_query_len we ever give this. */
@@ -671,6 +646,8 @@ innobase_mysql_print_thd(
671646
}
672647
}
673648

649+
VOID(pthread_mutex_unlock(&thd->LOCK_thd_data));
650+
674651
putc('\n', f);
675652
}
676653

0 commit comments

Comments
 (0)