List:Commits« Previous MessageNext Message »
From:Staale Smedseng Date:October 16 2009 2:54pm
Subject:bzr commit into mysql-5.1-bugteam branch (staale.smedseng:3153) Bug#45058
View as plain text  
#At file:///net/atum21.norway/export/home/tmp/ss156133/z/45058-51/ based on revid:magnus.blaudd@stripped

 3153 Staale Smedseng	2009-10-16
      Bug #45058 init_available_charsets uses double checked locking
            
      As documented in the bug report, the double checked locking
      pattern has inherent issues, and cannot guarantee correct
      initialization.
      
      This is a new patch with an alternative solution, intended to
      avoid any possible API incompatibilities introduced in the
      initial patch.
      
      This patch replaces the logic in init_available_charsets()
      with the use of pthread_once(3). A wrapper function,
      my_pthread_once(), is introduced and is used in lieu of direct
      calls to init_available_charsets().
      
      For the Windows platform, the implementation in lp:sysbench is
      ported. For single-thread use, a simple define calls the
      function and sets the pthread_once control variable.
     @ include/my_no_pthread.h
        Dummy my_pthread_once() for single thread use.
     @ include/my_pthread.h
        Declaration for new function my_pthread_once().
     @ mysys/charset.c
        Logic in init_available_charsets() is simplified. 
        Using my_pthread_once() for all calls to this func.

    modified:
      include/my_no_pthread.h
      include/my_pthread.h
      mysys/charset.c
      mysys/my_winthread.c
      netware/libmysqlmain.c
=== modified file 'include/my_no_pthread.h'
--- a/include/my_no_pthread.h	2006-12-23 19:20:40 +0000
+++ b/include/my_no_pthread.h	2009-10-16 14:54:10 +0000
@@ -47,4 +47,8 @@
 #define rw_unlock(A)
 #define rwlock_destroy(A)
 
+#define PTHREAD_ONCE_DONE 2
+#define my_pthread_once(C,F)  \
+  do { if (*(C) != PTHREAD_ONCE_DONE) { F(); *(C)= PTHREAD_ONCE_DONE; } } while(0)
+
 #endif

=== modified file 'include/my_pthread.h'
--- a/include/my_pthread.h	2008-10-15 22:28:26 +0000
+++ b/include/my_pthread.h	2009-10-16 14:54:10 +0000
@@ -69,6 +69,11 @@ typedef int pthread_mutexattr_t;
 #define pthread_handler_t EXTERNC void * __cdecl
 typedef void * (__cdecl *pthread_handler)(void *);
 
+typedef volatile LONG pthread_once_t;
+#define PTHREAD_ONCE_INIT  0
+#define PTHREAD_ONCE_INPROGRESS 1
+#define PTHREAD_ONCE_DONE 2
+
 /*
   Struct and macros to be used in combination with the
   windows implementation of pthread_cond_timedwait
@@ -114,6 +119,7 @@ int pthread_attr_init(pthread_attr_t *co
 int pthread_attr_setstacksize(pthread_attr_t *connect_att,DWORD stack);
 int pthread_attr_setprio(pthread_attr_t *connect_att,int priority);
 int pthread_attr_destroy(pthread_attr_t *connect_att);
+int my_pthread_once(pthread_once_t *once_control,void (*init_routine)(void));
 struct tm *localtime_r(const time_t *timep,struct tm *tmp);
 struct tm *gmtime_r(const time_t *timep,struct tm *tmp);
 
@@ -209,6 +215,7 @@ extern int my_pthread_getprio(pthread_t 
 #define pthread_detach_this_thread()
 #define pthread_handler_t EXTERNC void *
 typedef void *(* pthread_handler)(void *);
+#define my_pthread_once(C,F) pthread_once(C,F)
 
 /* Test first for RTS or FSU threads */
 

=== modified file 'mysys/charset.c'
--- a/mysys/charset.c	2009-06-10 08:59:49 +0000
+++ b/mysys/charset.c	2009-10-16 14:54:10 +0000
@@ -399,51 +399,31 @@ static void *cs_alloc(size_t size)
 }
 
 
-#ifdef __NETWARE__
-my_bool STDCALL init_available_charsets(myf myflags)
-#else
-static my_bool init_available_charsets(myf myflags)
-#endif
+static pthread_once_t charsets_initialized= PTHREAD_ONCE_INIT;
+
+static void init_available_charsets(void)
 {
   char fname[FN_REFLEN + sizeof(MY_CHARSET_INDEX)];
-  my_bool error=FALSE;
-  /*
-    We have to use charset_initialized to not lock on THR_LOCK_charset
-    inside get_internal_charset...
-  */
-  if (!charset_initialized)
+  CHARSET_INFO **cs;
+
+  bzero(&all_charsets,sizeof(all_charsets));
+  init_compiled_charsets(MYF(0));
+      
+  /* Copy compiled charsets */
+  for (cs=all_charsets;
+       cs < all_charsets+array_elements(all_charsets)-1 ;
+       cs++)
   {
-    CHARSET_INFO **cs;
-    /*
-      To make things thread safe we are not allowing other threads to interfere
-      while we may changing the cs_info_table
-    */
-    pthread_mutex_lock(&THR_LOCK_charset);
-    if (!charset_initialized)
+    if (*cs)
     {
-      bzero(&all_charsets,sizeof(all_charsets));
-      init_compiled_charsets(myflags);
-      
-      /* Copy compiled charsets */
-      for (cs=all_charsets;
-           cs < all_charsets+array_elements(all_charsets)-1 ;
-           cs++)
-      {
-        if (*cs)
-        {
-          if (cs[0]->ctype)
-            if (init_state_maps(*cs))
-              *cs= NULL;
-        }
-      }
-      
-      strmov(get_charsets_dir(fname), MY_CHARSET_INDEX);
-      error= my_read_charset_file(fname,myflags);
-      charset_initialized=1;
+      if (cs[0]->ctype)
+        if (init_state_maps(*cs))
+          *cs= NULL;
     }
-    pthread_mutex_unlock(&THR_LOCK_charset);
   }
-  return error;
+      
+  strmov(get_charsets_dir(fname), MY_CHARSET_INDEX);
+  my_read_charset_file(fname, MYF(0));
 }
 
 
@@ -455,7 +435,7 @@ void free_charsets(void)
 
 uint get_collation_number(const char *name)
 {
-  init_available_charsets(MYF(0));
+  my_pthread_once(&charsets_initialized, init_available_charsets);
   return get_collation_number_internal(name);
 }
 
@@ -463,7 +443,7 @@ uint get_collation_number(const char *na
 uint get_charset_number(const char *charset_name, uint cs_flags)
 {
   CHARSET_INFO **cs;
-  init_available_charsets(MYF(0));
+  my_pthread_once(&charsets_initialized, init_available_charsets);
   
   for (cs= all_charsets;
        cs < all_charsets+array_elements(all_charsets)-1 ;
@@ -480,7 +460,7 @@ uint get_charset_number(const char *char
 const char *get_charset_name(uint charset_number)
 {
   CHARSET_INFO *cs;
-  init_available_charsets(MYF(0));
+  my_pthread_once(&charsets_initialized, init_available_charsets);
 
   cs=all_charsets[charset_number];
   if (cs && (cs->number == charset_number) && cs->name )
@@ -538,7 +518,7 @@ CHARSET_INFO *get_charset(uint cs_number
   if (cs_number == default_charset_info->number)
     return default_charset_info;
 
-  (void) init_available_charsets(MYF(0));	/* If it isn't initialized */
+  my_pthread_once(&charsets_initialized, init_available_charsets);
   
   if (!cs_number || cs_number >= array_elements(all_charsets)-1)
     return NULL;
@@ -560,7 +540,7 @@ CHARSET_INFO *get_charset_by_name(const 
 {
   uint cs_number;
   CHARSET_INFO *cs;
-  (void) init_available_charsets(MYF(0));	/* If it isn't initialized */
+  my_pthread_once(&charsets_initialized, init_available_charsets);
 
   cs_number=get_collation_number(cs_name);
   cs= cs_number ? get_internal_charset(cs_number,flags) : NULL;
@@ -585,7 +565,7 @@ CHARSET_INFO *get_charset_by_csname(cons
   DBUG_ENTER("get_charset_by_csname");
   DBUG_PRINT("enter",("name: '%s'", cs_name));
 
-  (void) init_available_charsets(MYF(0));	/* If it isn't initialized */
+  my_pthread_once(&charsets_initialized, init_available_charsets);
 
   cs_number= get_charset_number(cs_name, cs_flags);
   cs= cs_number ? get_internal_charset(cs_number, flags) : NULL;

=== modified file 'mysys/my_winthread.c'
--- a/mysys/my_winthread.c	2007-10-03 19:38:32 +0000
+++ b/mysys/my_winthread.c	2009-10-16 14:54:10 +0000
@@ -137,4 +137,36 @@ int win_pthread_setspecific(void *a,void
   return 0;
 }
 
+
+/*
+ One time initialization. For simplicity, we assume initializer thread
+ does not exit within init_routine().
+*/
+int my_pthread_once(pthread_once_t *once_control, 
+    void (*init_routine)(void))
+{
+  LONG state= InterlockedCompareExchange(once_control, PTHREAD_ONCE_INPROGRESS,
+                                          PTHREAD_ONCE_INIT);
+  switch(state)
+  {
+  case PTHREAD_ONCE_INIT:
+    /* This is initializer thread */
+    (*init_routine)();
+    *once_control= PTHREAD_ONCE_DONE;
+    break;
+
+  case PTHREAD_ONCE_INPROGRESS:
+    /* init_routine in progress. Wait for its completion */
+    while(*once_control == PTHREAD_ONCE_INPROGRESS)
+    {
+      Sleep(1);
+    }
+    break;
+  case PTHREAD_ONCE_DONE:
+    /* Nothing to do */
+    break;
+  }
+  return 0;
+}
+
 #endif

=== modified file 'netware/libmysqlmain.c'
--- a/netware/libmysqlmain.c	2003-01-31 23:42:26 +0000
+++ b/netware/libmysqlmain.c	2009-10-16 14:54:10 +0000
@@ -18,7 +18,7 @@
 
 #include "my_global.h"
 
-my_bool init_available_charsets(myf myflags);
+void init_available_charsets(void);
 
 /* this function is required so that global memory is allocated against this
 library nlm, and not against a paticular client */
@@ -31,7 +31,7 @@ int _NonAppStart(void *NLMHandle, void *
 {
   mysql_server_init(0, NULL, NULL);
   
-  init_available_charsets(MYF(0));
+  init_available_charsets();
 
   return 0;
 }


Attachment: [text/bzr-bundle] bzr/staale.smedseng@sun.com-20091016145410-d1cq2ur7b0fis56s.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (staale.smedseng:3153) Bug#45058Staale Smedseng16 Oct
  • Re: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:3153)Bug#45058Konstantin Osipov16 Oct
  • Re: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:3153)Bug#45058Davi Arnaut17 Nov