List:Commits« Previous MessageNext Message »
From:Christopher Powers Date:July 30 2012 11:53pm
Subject:bzr push into mysql-trunk branch (chris.powers:4117 to 4118) Bug#13915200
View as plain text  
 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#13915200Christopher Powers31 Jul