4118 Christopher Powers 2012-07-27
Bug#13915200 - SET GLOBAL DEBUG CAUSES CRASHES IN DBUG.C, RACE CONDITIONS
Original fix with fixes for Windows compile
modified:
dbug/dbug.c
4117 Gopal Shankar 2012-07-25 [merge]
Merged with 5.6
=== modified file 'dbug/dbug.c'
--- a/dbug/dbug.c 2012-06-19 08:28:27 +0000
+++ b/dbug/dbug.c 2012-07-27 19:28:41 +0000
@@ -236,6 +236,12 @@ struct settings {
static BOOLEAN init_done= FALSE; /* Set to TRUE when initialization done */
+/**
+ Global debugging settings.
+ This structure shared between all threads,
+ and is the last element in each thread @c CODE_STATE::stack chain.
+ Protected by @c THR_LOCK_init_settings.
+*/
static struct settings init_settings;
static const char *db_process= 0;/* Pointer to process name; argv[0] */
my_bool _dbug_on_= TRUE; /* FALSE if no debugging at all */
@@ -265,6 +271,7 @@ typedef struct _db_code_state_ {
uint u_line; /* User source code line number */
int locked; /* If locked with _db_lock_file_ */
const char *u_keyword; /* Keyword for current macro */
+ uint m_read_lock_count;
} CODE_STATE;
/*
@@ -324,6 +331,7 @@ static void DbugVfprintf(FILE *stream, c
*/
#define ERR_MISSING_RETURN "missing DBUG_RETURN or DBUG_VOID_RETURN macro in function \"%s\"\n"
+#define ERR_MISSING_UNLOCK "missing DBUG_UNLOCK_FILE macro in function \"%s\"\n"
#define ERR_OPEN "%s: can't open debug output stream \"%s\": "
#define ERR_CLOSE "%s: can't close debug file: "
#define ERR_ABORT "%s: debugger aborting because %s\n"
@@ -348,6 +356,13 @@ static void DbugVfprintf(FILE *stream, c
#include <my_pthread.h>
static pthread_mutex_t THR_LOCK_dbug;
+/**
+ Lock, to protect @c init_settings.
+ For performance reasons,
+ the member @c init_settings.flags is not protected.
+*/
+//static pthread_rwlock_t THR_LOCK_init_settings;
+static my_rw_lock_t THR_LOCK_init_settings;
static CODE_STATE *code_state(void)
{
@@ -364,6 +379,7 @@ static CODE_STATE *code_state(void)
{
init_done=TRUE;
pthread_mutex_init(&THR_LOCK_dbug, NULL);
+ my_rwlock_init(&THR_LOCK_init_settings, NULL);
memset(&init_settings, 0, sizeof(init_settings));
init_settings.out_file=stderr;
init_settings.flags=OPEN_APPEND;
@@ -379,11 +395,41 @@ static CODE_STATE *code_state(void)
cs->func="?func";
cs->file="?file";
cs->stack=&init_settings;
+ cs->m_read_lock_count= 0;
*cs_ptr= cs;
}
return cs;
}
+/**
+ Lock the stack debugging settings.
+ Only the shared (global) settings are locked if necessary,
+ per thread settings are local and safe to use.
+ This lock is re entrant.
+ @sa unlock_stack
+*/
+static void read_lock_stack(CODE_STATE *cs)
+{
+ if (cs->stack == &init_settings)
+ {
+ if (++(cs->m_read_lock_count) == 1)
+ rw_rdlock(&THR_LOCK_init_settings);
+ }
+}
+
+/**
+ Unlock the stack debugging settings.
+ @sa read_lock_stack
+*/
+static void unlock_stack(CODE_STATE *cs)
+{
+ if (cs->stack == &init_settings)
+ {
+ if (--(cs->m_read_lock_count) == 0)
+ rw_unlock(&THR_LOCK_init_settings);
+ }
+}
+
/*
* Translate some calls among different systems.
*/
@@ -454,8 +500,25 @@ int DbugParse(CODE_STATE *cs, const char
int rel, f_used=0;
struct settings *stack;
+ /*
+ Make sure we are not changing settings while inside a
+ DBUG_LOCK_FILE
+ DBUG_UNLOCK_FILE
+ section, that is a mis use, that would cause changing
+ DBUG_FILE while the caller prints to it.
+ */
+ assert(! cs->locked);
+
stack= cs->stack;
+ /*
+ When parsing the global init_settings itself,
+ make sure to block every other thread using dbug functions.
+ */
+ assert(cs->m_read_lock_count == 0);
+ if (stack == &init_settings)
+ rw_wrlock(&THR_LOCK_init_settings);
+
if (control[0] == '-' && control[1] == '#')
control+=2;
@@ -485,6 +548,9 @@ int DbugParse(CODE_STATE *cs, const char
stack->prof_file= stack->next->prof_file;
if (stack->next == &init_settings)
{
+ assert(stack != &init_settings);
+ rw_rdlock(&THR_LOCK_init_settings);
+
/*
Never share with the global parent - it can change under your feet.
@@ -496,6 +562,8 @@ int DbugParse(CODE_STATE *cs, const char
stack->p_functions= ListCopy(init_settings.p_functions);
stack->keywords= ListCopy(init_settings.keywords);
stack->processes= ListCopy(init_settings.processes);
+
+ rw_unlock(&THR_LOCK_init_settings);
}
else
{
@@ -664,6 +732,21 @@ int DbugParse(CODE_STATE *cs, const char
control=end+1;
end= DbugStrTok(control);
}
+
+ if (stack->next == &init_settings)
+ {
+ /*
+ Enforce nothing is shared with the global init_settings
+ */
+ assert((stack->functions == NULL) || (stack->functions != init_settings.functions));
+ assert((stack->p_functions == NULL) || (stack->p_functions != init_settings.p_functions));
+ assert((stack->keywords == NULL) || (stack->keywords != init_settings.keywords));
+ assert((stack->processes == NULL) || (stack->processes != init_settings.processes));
+ }
+
+ if (stack == &init_settings)
+ rw_unlock(&THR_LOCK_init_settings);
+
return !rel || f_used;
}
@@ -794,11 +877,20 @@ void _db_set_(const char *control)
CODE_STATE *cs;
uint old_fflags;
get_code_state_or_return;
+
+ read_lock_stack(cs);
old_fflags=fflags(cs);
+ unlock_stack(cs);
+
if (cs->stack == &init_settings)
PushState(cs);
+
if (DbugParse(cs, control))
+ {
+ read_lock_stack(cs);
FixTraceFlags(old_fflags, cs);
+ unlock_stack(cs);
+ }
}
/*
@@ -824,10 +916,19 @@ void _db_push_(const char *control)
CODE_STATE *cs;
uint old_fflags;
get_code_state_or_return;
+
+ read_lock_stack(cs);
old_fflags=fflags(cs);
+ unlock_stack(cs);
+
PushState(cs);
+
if (DbugParse(cs, control))
+ {
+ read_lock_stack(cs);
FixTraceFlags(old_fflags, cs);
+ unlock_stack(cs);
+ }
}
@@ -894,10 +995,16 @@ void _db_pop_()
discard= cs->stack;
if (discard != &init_settings)
{
+ read_lock_stack(cs);
old_fflags=fflags(cs);
+ unlock_stack(cs);
+
cs->stack= discard->next;
FreeState(cs, discard, 1);
+
+ read_lock_stack(cs);
FixTraceFlags(old_fflags, cs);
+ unlock_stack(cs);
}
}
@@ -999,6 +1106,8 @@ int _db_explain_ (CODE_STATE *cs, char *
get_code_state_if_not_set_or_return *buf=0;
+ read_lock_stack(cs);
+
op_list_to_buf('d', cs->stack->keywords, DEBUGGING);
op_int_to_buf ('D', cs->stack->delay, 0);
op_list_to_buf('f', cs->stack->functions, cs->stack->functions);
@@ -1018,6 +1127,8 @@ int _db_explain_ (CODE_STATE *cs, char *
op_intf_to_buf('t', cs->stack->maxdepth, MAXDEPTH, TRACING);
op_bool_to_buf('T', cs->stack->flags & TIMESTAMP_ON);
+ unlock_stack(cs);
+
*buf= '\0';
return 0;
@@ -1026,6 +1137,8 @@ overflow:
*end++= '.';
*end++= '.';
*end= '\0';
+
+ unlock_stack(cs);
return 1;
}
@@ -1104,6 +1217,8 @@ void _db_enter_(const char *_func_, cons
}
save_errno= errno;
+ read_lock_stack(cs);
+
_stack_frame_->func= cs->func;
_stack_frame_->file= cs->file;
cs->func= _func_;
@@ -1135,6 +1250,8 @@ void _db_enter_(const char *_func_, cons
break;
}
errno=save_errno;
+
+ unlock_stack(cs);
}
/*
@@ -1171,6 +1288,8 @@ void _db_return_(uint _line_, struct _db
DbugExit(buf);
}
+ read_lock_stack(cs);
+
if (DoTrace(cs) & DO_TRACE)
{
if (TRACING)
@@ -1193,6 +1312,8 @@ void _db_return_(uint _line_, struct _db
if (cs->framep != NULL)
cs->framep= cs->framep->prev;
errno=save_errno;
+
+ unlock_stack(cs);
}
@@ -1257,7 +1378,12 @@ void _db_doprnt_(const char *format,...)
CODE_STATE *cs;
get_code_state_or_return;
+ /* Dirty read, for DBUG_PRINT() performance. */
+ if (! DEBUGGING)
+ return;
+
va_start(args,format);
+ read_lock_stack(cs);
if (_db_keyword_(cs, cs->u_keyword, 0))
{
@@ -1274,6 +1400,7 @@ void _db_doprnt_(const char *format,...)
DbugFlush(cs);
errno=save_errno;
}
+ unlock_stack(cs);
va_end(args);
}
@@ -1315,6 +1442,12 @@ void _db_dump_(uint _line_, const char *
CODE_STATE *cs;
get_code_state_or_return;
+ /* Dirty read, for DBUG_DUMP() performance. */
+ if (! DEBUGGING)
+ return;
+
+ read_lock_stack(cs);
+
if (_db_keyword_(cs, keyword, 0))
{
if (!cs->locked)
@@ -1348,6 +1481,8 @@ void _db_dump_(uint _line_, const char *
(void) fputc('\n',cs->stack->out_file);
DbugFlush(cs);
}
+
+ unlock_stack(cs);
}
@@ -1625,6 +1760,17 @@ void _db_end_()
_dbug_on_= 1;
get_code_state_or_return;
+ /*
+ The caller may have missed a DBUG_UNLOCK_FILE,
+ we are breaking this lock to enforce DBUG_END can proceed.
+ */
+ if (cs->locked)
+ {
+ fprintf(stderr, ERR_MISSING_UNLOCK, "(unknown)");
+ cs->locked= 0;
+ pthread_mutex_unlock(&THR_LOCK_dbug);
+ }
+
while ((discard= cs->stack))
{
if (discard == &init_settings)
@@ -1632,10 +1778,9 @@ void _db_end_()
cs->stack= discard->next;
FreeState(cs, discard, 1);
}
- tmp= init_settings;
- /* Use mutex lock to make it less likely anyone access out_file */
- pthread_mutex_lock(&THR_LOCK_dbug);
+ rw_wrlock(&THR_LOCK_init_settings);
+ tmp= init_settings;
init_settings.flags= OPEN_APPEND;
init_settings.out_file= stderr;
init_settings.prof_file= stderr;
@@ -1646,7 +1791,7 @@ void _db_end_()
init_settings.p_functions= 0;
init_settings.keywords= 0;
init_settings.processes= 0;
- pthread_mutex_unlock(&THR_LOCK_dbug);
+ rw_unlock(&THR_LOCK_init_settings);
FreeState(cs, &tmp, 0);
}
@@ -1711,11 +1856,22 @@ FILE *_db_fp_(void)
BOOLEAN _db_keyword_(CODE_STATE *cs, const char *keyword, int strict)
{
+ BOOLEAN result;
get_code_state_if_not_set_or_return FALSE;
+
+ /* Dirty read, for DBUG_EXECUTE(), DBUG_EXECUTE_IF() ... performance. */
+ if (! DEBUGGING)
+ return FALSE;
+
+ read_lock_stack(cs);
+
strict=strict ? INCLUDE : INCLUDE|MATCHED;
+ result= DoTrace(cs) & DO_TRACE &&
+ InList(cs->stack->keywords, keyword) & strict;
- return DEBUGGING && DoTrace(cs) & DO_TRACE &&
- InList(cs->stack->keywords, keyword) & strict;
+ unlock_stack(cs);
+
+ return result;
}
/*
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (chris.powers:4117 to 4118) Bug#13915200 | Christopher Powers | 31 Jul |
