List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:December 3 2008 9:33pm
Subject:bzr commit into mysql-5.1 branch (vvaintroub:2709) Bug#38522
View as plain text  
#At file:///G:/bzr/mysql-5.1-bugteam/

 2709 Vladislav Vaintroub	2008-12-03
      Bug#38522: 5 seconds delay when closing application using embedded server
                  
      The problem here is that embedded server starts handle_thread manager 
      thread  on mysql_library_init() does not stop it on mysql_library_end().
      At shutdown, my_thread_global_end() waits for thread count to become 0,
      but since we did not stop the thread it will give up after 5 seconds.
             
      Solution is to move shutdown for handle_manager thread from kill_server()
      (mysqld specific) to clean_up() that is used by both embedded and mysqld.
            
      This patch also contains some refactorings - to avoid duplicate code,
      start_handle_manager() and stop_handle_manager() functions are introduced.
      Unused variables are eliminated. handle_manager does not rely on global
      variable abort_loop anymore to stop (abort_loop is not set for embedded).
            
      Note: Specifically on Windows and when using DBUG version of libmysqld, 
      the complete solution requires removing obsolete code my_thread_init() 
      from my_thread_var(). This has a side effect that a DBUG statement 
      after my_thread_end() can cause thread counter to be incremented, and 
      embedded will hang for some seconds. Or worse, my_thread_init() will 
      crash if critical sections have been deleted by the global cleanup 
      routine that runs in a different thread.
modified:
  client/mysql.cc
  dbug/dbug.c
  include/my_dbug.h
  libmysql/libmysql.c
  libmysqld/examples/CMakeLists.txt
  libmysqld/lib_sql.cc
  libmysqld/libmysqld.def
  mysys/my_thr_init.c
  sql/mysql_priv.h
  sql/mysqld.cc
  sql/sql_manager.cc

per-file messages:
  client/mysql.cc
    sql_protocol_typelib is not exported from libmysqld
    (and does not make sense either)
    thus excluded from embedded client
  dbug/dbug.c
    revert changes for Bug#38293
  include/my_dbug.h
    revert changes for Bug#38293
  libmysql/libmysql.c
    Removed DBUG_POP call, because when called after my_end(), will access
    THR_key_mysys that is already deleted. The result of pthread_get_specific
    is not predictable in this case and hence DBUG_POP can crash.
  libmysqld/examples/CMakeLists.txt
    Revert changes for Bug#38293.
    
    client/get_password.c is now compiled with every test executable
    (because of get_tty_password, which logically  belongs to 
    places in mysys
  libmysqld/libmysqld.def
    Revert changes for Bug #38293
    Remove excessive exports from libmysqld, export only what's documented.
    (mysql_xxx functions)
  mysys/my_thr_init.c
    Remove windows-DLL-specific workaround for something (old code, no documentation for
    what specifically). The problem is that even after
    my_thread_end() is finished, DBUG statement can initiate my_thread_init(). This does
    not happen anywhere else and should not happen on  Windows either.
  sql/mysql_priv.h
    - new functions start_handle_manager() and stop_handle_manager()
    - move manager_thread_in_use  variable to sql_manager.cc and made
    it static
    - remove manager_status, as it is unused
  sql/mysqld.cc
    Code to start/stop handle_manager thread is factored out into start_handle_manager()
  sql/sql_manager.cc
    - new functions start_handle_manager() and stop_handle_manager().
    - removed unused status and manager status variable.
    - don't rely on global abort_loop variable to stop the thread,
     (not set in embedded), use static abort_manager that is set
        by stop_handle_manager()
    - changed DBUG_RETURN in handle_manager() to DBUG_LEAVE + return.
    Using DBUG_RETURN is unsafe here, it can access TLS key that is already deleted in
    my_thread_global_end()
=== modified file 'client/mysql.cc'
--- a/client/mysql.cc	2008-07-22 17:41:26 +0000
+++ b/client/mysql.cc	2008-12-03 21:33:28 +0000
@@ -1626,10 +1626,12 @@ get_one_option(int optid, const struct m
     printf("WARNING: option deprecated; use --disable-pager instead.\n");
     opt_nopager= 1;
     break;
+#ifndef EMBEDDED_LIBRARY
   case OPT_MYSQL_PROTOCOL:
     opt_protocol= find_type_or_exit(argument, &sql_protocol_typelib,
                                     opt->name);
     break;
+#endif
   case OPT_SERVER_ARG:
 #ifdef EMBEDDED_LIBRARY
     /*

=== modified file 'dbug/dbug.c'
--- a/dbug/dbug.c	2008-11-19 09:57:23 +0000
+++ b/dbug/dbug.c	2008-12-03 21:33:28 +0000
@@ -1851,13 +1851,7 @@ static void DBUGOpenFile(CODE_STATE *cs,
       else
       {
         newfile= !EXISTS(name);
-        if (!(fp= fopen(name,
-#if defined(MSDOS) || defined(__WIN__)
-		append ? "a+c" : "wc"
-#else
-                append ? "a+" : "w"
-#endif
-		)))
+        if (!(fp= fopen(name, append ? "a+" : "w")))
         {
           (void) fprintf(stderr, ERR_OPEN, cs->process, name);
           perror("");
@@ -2407,13 +2401,4 @@ int i_am_a_dummy_function() {
        return 0;
 }
 
-#ifdef __WIN__
-char _db_doprnt_;
-char _db_enter_;
-char _db_pargs_;
-char _db_process_;
-char _db_push_;
-char _db_return_;
-#endif /*__WIN__*/
-
 #endif

=== modified file 'include/my_dbug.h'
--- a/include/my_dbug.h	2008-11-19 09:57:23 +0000
+++ b/include/my_dbug.h	2008-12-03 21:33:28 +0000
@@ -109,21 +109,6 @@ extern FILE *_db_fp_(void);
 #define DBUG_EXPLAIN_INITIAL(buf,len)
 #define IF_DBUG(A)
 
-#ifdef __WIN__
-/*
-   On windows all the dll export has to be declared in the *.def file
-   so as we export these symbols in DEBUG mode we have to export
-   these in the RELEASE mode also. So below are the dummy symbols
-   for the RELEASE export
-*/
-extern  char _db_doprnt_;
-extern  char _db_enter_;
-extern  char _db_pargs_;
-extern  char _db_process_;
-extern  char _db_push_;
-extern  char _db_return_;
-#endif /*__WIN__*/
-
 #endif
 #ifdef	__cplusplus
 }

=== modified file 'libmysql/libmysql.c'
--- a/libmysql/libmysql.c	2008-05-20 16:36:26 +0000
+++ b/libmysql/libmysql.c	2008-12-03 21:33:28 +0000
@@ -207,9 +207,7 @@ void STDCALL mysql_server_end()
   /* If library called my_init(), free memory allocated by it */
   if (!org_my_init_done)
   {
-    my_end(MY_DONT_FREE_DBUG);
-    /* Remove TRACING, if enabled by mysql_debug() */
-    DBUG_POP();
+    my_end(0);
   }
   else
   {

=== modified file 'libmysqld/examples/CMakeLists.txt'
--- a/libmysqld/examples/CMakeLists.txt	2008-11-19 11:51:31 +0000
+++ b/libmysqld/examples/CMakeLists.txt	2008-12-03 21:33:28 +0000
@@ -28,14 +28,14 @@ ADD_DEFINITIONS(-DEMBEDDED_LIBRARY)
 
 ADD_EXECUTABLE(mysql_embedded ../../client/completion_hash.cc
                ../../client/mysql.cc ../../client/readline.cc
-               ../../client/sql_string.cc)
-TARGET_LINK_LIBRARIES(mysql_embedded wsock32)
+               ../../client/sql_string.cc ../../client/get_password.c)
+TARGET_LINK_LIBRARIES(mysql_embedded mysys yassl taocrypt zlib debug dbug regex strings ws2_32)
 ADD_DEPENDENCIES(mysql_embedded libmysqld)
 
-ADD_EXECUTABLE(mysqltest_embedded ../../client/mysqltest.c)
-TARGET_LINK_LIBRARIES(mysqltest_embedded wsock32)
+ADD_EXECUTABLE(mysqltest_embedded ../../client/mysqltest.c ../../client/get_password.c )
+TARGET_LINK_LIBRARIES(mysqltest_embedded mysys yassl taocrypt zlib debug dbug regex strings ws2_32)
 ADD_DEPENDENCIES(mysqltest_embedded libmysqld)
 
-ADD_EXECUTABLE(mysql_client_test_embedded ../../tests/mysql_client_test.c)
-TARGET_LINK_LIBRARIES(mysql_client_test_embedded wsock32)
+ADD_EXECUTABLE(mysql_client_test_embedded ../../tests/mysql_client_test.c ../../client/get_password.c)
+TARGET_LINK_LIBRARIES(mysql_client_test_embedded debug dbug mysys yassl taocrypt zlib strings ws2_32 )
 ADD_DEPENDENCIES(mysql_client_test_embedded libmysqld)

=== modified file 'libmysqld/lib_sql.cc'
--- a/libmysqld/lib_sql.cc	2008-11-17 16:06:03 +0000
+++ b/libmysqld/lib_sql.cc	2008-12-03 21:33:28 +0000
@@ -539,12 +539,7 @@ int init_embedded_server(int argc, char 
 
   (void) thr_setconcurrency(concurrency);	// 10 by default
 
-  if (flush_time && flush_time != ~(ulong) 0L)
-  {
-    pthread_t hThread;
-    if (pthread_create(&hThread,&connection_attrib,handle_manager,0))
-      sql_print_error("Warning: Can't create thread to manage maintenance");
-  }
+  start_handle_manager();
 
   // FIXME initialize binlog_filter and rpl_filter if not already done
   //       corresponding delete is in clean_up()

=== modified file 'libmysqld/libmysqld.def'
--- a/libmysqld/libmysqld.def	2008-11-21 10:15:26 +0000
+++ b/libmysqld/libmysqld.def	2008-12-03 21:33:28 +0000
@@ -2,97 +2,8 @@ LIBRARY		LIBMYSQLD
 DESCRIPTION	'MySQL 5.1 Embedded Server Library'
 VERSION		5.1
 EXPORTS
-	_db_process_
-	_db_enter_
-	_db_return_
-	_db_push_
-	_db_doprnt_
-	_db_pargs_
-	strnmov
-	get_charset
-	my_memmem
-	my_snprintf
-	pthread_exit
-	pthread_cond_signal
-	dynstr_append_mem
-	init_dynamic_string
-	dynstr_free
-	my_hash_free
-	my_vsnprintf
-	dynstr_append
-	my_close
-	my_open
-	dynstr_set
-	dynstr_append_os_quoted
-	my_delete
-	my_seek
-	my_write
-	create_temp_file
-	fn_format
-	dirname_part
-	my_hash_insert
-	my_hash_search
-	test_if_hard_path
-	my_copy
-	my_mkdir
-	my_sleep
-	my_strtod
-	pthread_cond_wait
-	my_strnncoll_simple
-	get_dynamic
-	my_regerror
-	init_dynamic_array2
-	pthread_create
-	pthread_cond_init
-	my_regcomp
-	my_regexec
-	my_regex_end
-	my_regfree
-	longlong2str
-	my_set_exception_pointers
-	my_print_stacktrace
-	my_thread_stack_size
-	my_safe_print_str
-	my_stat
-	_my_hash_init
-	pthread_attr_setstacksize
-	pthread_attr_init
-	my_dirend
-	wild_compare
-	my_dir
-	my_micro_time
-	find_type_or_exit
-	_dig_vec_upper
-	_dig_vec_lower
-	bmove_upp
-	delete_dynamic
-	free_defaults
-	getopt_compare_strings
-	getopt_ull_limit_value
-	handle_options
-	init_dynamic_array
-	insert_dynamic
-	int2str
-	is_prefix
-	list_add
-	list_delete
-	load_defaults
-	max_allowed_packet
-	my_cgets
-	my_end
-	my_getopt_print_errors
-	my_init
-	my_malloc
-	my_memdup
-	my_no_flags_free
-	my_path
-	my_print_help
-	my_print_variables
-	my_realloc
-	my_strdup
 	mysql_thread_end
 	mysql_thread_init
-	myodbc_remove_escape
 	mysql_affected_rows
 	mysql_autocommit
 	mysql_change_user
@@ -162,44 +73,11 @@ EXPORTS
 	mysql_thread_safe
 	mysql_use_result
 	mysql_warning_count
-	set_dynamic
-	strcend
-	strcont
-	strdup_root
-	strfill
-	strinstr
-	strmake
-	strmov
-	strxmov
 	mysql_server_end
 	mysql_server_init
-	get_tty_password
-	sql_protocol_typelib
 	mysql_get_server_version
 	mysql_set_character_set
 	mysql_sqlstate
-	charsets_dir
-	disabled_my_option
-	my_charset_latin1
-	init_alloc_root
-	my_progname
-	get_charset_name
-	get_charset_by_csname
-	print_defaults
-	find_type
-	strxnmov
-	strend
-	my_fopen
-	my_fclose
-	unpack_filename
-	str2int
-	int10_to_str
-	longlong10_to_str
-	my_snprintf_8bit
-	alloc_root
-	free_root
-	my_read
-	llstr
 	mysql_get_parameters
 	mysql_thread_init
 	mysql_thread_end
@@ -230,7 +108,4 @@ EXPORTS
 	mysql_stmt_attr_get
 	mysql_stmt_attr_set
 	mysql_stmt_field_count
-	get_defaults_options
-	my_charset_bin
-	my_charset_same
-	modify_defaults_file
+

=== modified file 'mysys/my_thr_init.c'
--- a/mysys/my_thr_init.c	2008-04-01 09:21:09 +0000
+++ b/mysys/my_thr_init.c	2008-12-03 21:33:28 +0000
@@ -368,17 +368,7 @@ void my_thread_end(void)
 
 struct st_my_thread_var *_my_thread_var(void)
 {
-  struct st_my_thread_var *tmp=
-    my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
-#if defined(USE_TLS)
-  /* This can only happen in a .DLL */
-  if (!tmp)
-  {
-    my_thread_init();
-    tmp=my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
-  }
-#endif
-  return tmp;
+  return  my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
 }
 
 

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2008-10-02 12:53:08 +0000
+++ b/sql/mysql_priv.h	2008-12-03 21:33:28 +0000
@@ -1767,10 +1767,9 @@ int mysql_load(THD *thd, sql_exchange *e
 int write_record(THD *thd, TABLE *table, COPY_INFO *info);
 
 /* sql_manager.cc */
-extern ulong volatile manager_status;
-extern bool volatile manager_thread_in_use, mqh_used;
-extern pthread_t manager_thread;
-pthread_handler_t handle_manager(void *arg);
+extern bool volatile  mqh_used;
+void start_handle_manager();
+void stop_handle_manager();
 bool mysql_manager_submit(void (*action)());
 
 

=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2008-11-22 00:22:20 +0000
+++ b/sql/mysqld.cc	2008-12-03 21:33:28 +0000
@@ -784,16 +784,6 @@ static void close_connections(void)
   kill_cached_threads++;
   flush_thread_cache();
 
-  /* kill flush thread */
-  (void) pthread_mutex_lock(&LOCK_manager);
-  if (manager_thread_in_use)
-  {
-    DBUG_PRINT("quit", ("killing manager thread: 0x%lx",
-                        (ulong)manager_thread));
-   (void) pthread_cond_signal(&COND_manager);
-  }
-  (void) pthread_mutex_unlock(&LOCK_manager);
-
   /* kill connection thread */
 #if !defined(__WIN__) && !defined(__NETWARE__)
   DBUG_PRINT("quit", ("waiting for select thread: 0x%lx",
@@ -1196,6 +1186,7 @@ void clean_up(bool print_message)
   if (cleanup_done++)
     return; /* purecov: inspected */
 
+  stop_handle_manager();
   release_ddl_log();
 
   /*
@@ -4036,17 +4027,6 @@ server.");
 
 #ifndef EMBEDDED_LIBRARY
 
-static void create_maintenance_thread()
-{
-  if (flush_time && flush_time != ~(ulong) 0L)
-  {
-    pthread_t hThread;
-    if (pthread_create(&hThread,&connection_attrib,handle_manager,0))
-      sql_print_warning("Can't create thread to manage maintenance");
-  }
-}
-
-
 static void create_shutdown_thread()
 {
 #ifdef __WIN__
@@ -4361,7 +4341,7 @@ we force server id to 2, but this MySQL 
   execute_ddl_log_recovery();
 
   create_shutdown_thread();
-  create_maintenance_thread();
+  start_handle_manager();
 
   if (Events::init(opt_noacl))
     unireg_abort(1);

=== modified file 'sql/sql_manager.cc'
--- a/sql/sql_manager.cc	2007-05-10 09:59:39 +0000
+++ b/sql/sql_manager.cc	2008-12-03 21:33:28 +0000
@@ -23,8 +23,9 @@
 
 #include "mysql_priv.h"
 
-ulong volatile manager_status;
-bool volatile manager_thread_in_use;
+
+static bool volatile manager_thread_in_use;
+static bool abort_manager;
 
 pthread_t manager_thread;
 pthread_mutex_t LOCK_manager;
@@ -63,7 +64,6 @@ bool mysql_manager_submit(void (*action)
 pthread_handler_t handle_manager(void *arg __attribute__((unused)))
 {
   int error = 0;
-  ulong status;
   struct timespec abstime;
   bool reset_flush_time = TRUE;
   struct handler_cb *cb= NULL;
@@ -72,7 +72,6 @@ pthread_handler_t handle_manager(void *a
 
   pthread_detach_this_thread();
   manager_thread = pthread_self();
-  manager_status = 0;
   manager_thread_in_use = 1;
 
   for (;;)
@@ -87,16 +86,14 @@ pthread_handler_t handle_manager(void *a
 	set_timespec(abstime, flush_time);
         reset_flush_time = FALSE;
       }
-      while (!manager_status && (!error || error == EINTR) && !abort_loop)
+      while ((!error || error == EINTR) && !abort_manager)
         error= pthread_cond_timedwait(&COND_manager, &LOCK_manager, &abstime);
     }
     else
     {
-      while (!manager_status && (!error || error == EINTR) && !abort_loop)
+      while ((!error || error == EINTR) && !abort_manager)
         error= pthread_cond_wait(&COND_manager, &LOCK_manager);
     }
-    status = manager_status;
-    manager_status = 0;
     if (cb == NULL)
     {
       cb= cb_list;
@@ -104,7 +101,7 @@ pthread_handler_t handle_manager(void *a
     }
     pthread_mutex_unlock(&LOCK_manager);
 
-    if (abort_loop)
+    if (abort_manager)
       break;
 
     if (error == ETIMEDOUT || error == ETIME)
@@ -121,11 +118,42 @@ pthread_handler_t handle_manager(void *a
       my_free((uchar*)cb, MYF(0));
       cb= next;
     }
-
-    if (status)
-      DBUG_PRINT("error", ("manager did not handle something: %lx", status));
   }
   manager_thread_in_use = 0;
+  DBUG_LEAVE; // Can't use DBUG_RETURN after my_thread_end
   my_thread_end();
-  DBUG_RETURN(NULL);
+  return (NULL);
 }
+
+
+/* Start handle manager thread */
+void start_handle_manager()
+{
+  DBUG_ENTER("start_handle_manager");
+  abort_manager = false;
+  if (flush_time && flush_time != ~(ulong) 0L)
+  {
+    pthread_t hThread;
+    if (pthread_create(&hThread,&connection_attrib,handle_manager,0))
+      sql_print_warning("Can't create handle_manager thread");
+  }
+  DBUG_VOID_RETURN;
+}
+
+
+/* Initiate shutdown of handle manager thread */
+void stop_handle_manager()
+{
+  DBUG_ENTER("stop_handle_manager");
+  abort_manager = true;
+  pthread_mutex_lock(&LOCK_manager);
+  if (manager_thread_in_use)
+  {
+    DBUG_PRINT("quit", ("initiate shutdown of handle manager thread: 0x%lx",
+                        (ulong)manager_thread));
+   pthread_cond_signal(&COND_manager);
+  }
+  pthread_mutex_unlock(&LOCK_manager);
+  DBUG_VOID_RETURN;
+}
+

Thread
bzr commit into mysql-5.1 branch (vvaintroub:2709) Bug#38522Vladislav Vaintroub3 Dec