List:Commits« Previous MessageNext Message »
From:kevin.lewis Date:December 27 2012 8:09pm
Subject:bzr push into mysql-trunk branch (kevin.lewis:5304 to 5305)
View as plain text  
 5305 kevin.lewis@stripped	2012-12-27
      Bug14762796-THREADEXIT() ON WINDOWS IS CALLED PREMATURELY
      WITH HANDLES NOT CLOSED
      
      InnoDB uses os_thread_create() as a pthread implementation.
      It pays no attention to what it returns on Windows.  That is
      because on non-Windows platforms it returns the same value as
      the third parameter, the thread_id.  If os_thread_create()
      actually fails to create a thread it does an exit(1).
      So from a *nix perspective, there is no need to check the
      return value.
      
      But on Windows, the return value is a handle to the thread object.
      This system thread object is held open by that handle even after
      the thread exits, until the process exits.  InnoDB does not close
      any os_thread_t handles since it make no attempt to record them.
      Until FTS, there was really no need to close these handles because
      all threads started by InnoDB live until shutdown.
      
      But FTS introduced some short-lived threads. These threads were
      left open until shutdown which was fixed by Bug14759163 by explicitly
      closing those threads.
      
      This patch is a general fix for closing Windows thread handles by
      allowing os_thread_create() to close it immediately after CreateThread()
      starts the thread. The thread object then stays around until the thread
      exits.  Then instead of returning this closed handle, os_thread_create()
      is redesigned to return void, recognizing that InnoDB code never checks
      that return value anyway.  So the schizophrenic type os_thread_t which
      was a thread_id on *nix and a HANDLE on Windows is just deleted.
      
      Approved by Jimmy in RB:1416

    modified:
      storage/innobase/include/os0thread.h
      storage/innobase/include/row0ftsort.h
      storage/innobase/log/log0recv.cc
      storage/innobase/os/os0thread.cc
      storage/innobase/row/row0ftsort.cc
 5304 Nirbhay Choubey	2012-12-27 [merge]
      Merge of patch for Bug#16046140 from mysql-5.6.

    modified:
      scripts/mysqld_safe.sh
=== modified file 'storage/innobase/include/os0thread.h'
--- a/storage/innobase/include/os0thread.h	revid:nirbhay.choubey@stripped
+++ b/storage/innobase/include/os0thread.h	revid:kevin.lewis@stripped
@@ -42,7 +42,6 @@ can wait inside InnoDB */
 #define OS_THREAD_PRIORITY_ABOVE_NORMAL	3
 
 #ifdef __WIN__
-typedef void*			os_thread_t;
 typedef DWORD			os_thread_id_t;	/*!< In Windows the thread id
 						is an unsigned long int */
 extern "C"  {
@@ -61,8 +60,7 @@ don't access the arguments and don't ret
 
 #else
 
-typedef pthread_t		os_thread_t;
-typedef os_thread_t		os_thread_id_t;	/*!< In Unix we use the thread
+typedef pthread_t		os_thread_id_t;	/*!< In Unix we use the thread
 						handle itself as the id of
 						the thread */
 extern "C"  { typedef void*	(*os_thread_func_t)(void*); }
@@ -101,13 +99,13 @@ os_thread_pf(
 	os_thread_id_t	a);	/*!< in: OS thread identifier */
 /****************************************************************//**
 Creates a new thread of execution. The execution starts from
-the function given. The start function takes a void* parameter
-and returns a ulint.
+the function given.
 NOTE: We count the number of threads in os_thread_exit(). A created
-thread should always use that to exit and not use return() to exit.
-@return	handle to the thread */
+thread should always use that to exit so thatthe thread count will be
+decremented.
+We do not return an error code because if there is one, we crash here. */
 UNIV_INTERN
-os_thread_t
+void
 os_thread_create_func(
 /*==================*/
 	os_thread_func_t	func,		/*!< in: pointer to function

=== modified file 'storage/innobase/include/row0ftsort.h'
--- a/storage/innobase/include/row0ftsort.h	revid:nirbhay.choubey@stripped
+++ b/storage/innobase/include/row0ftsort.h	revid:kevin.lewis@stripped
@@ -87,7 +87,6 @@ struct fts_psort_t {
 	ulint			state;		/*!< child thread state */
 	fts_doc_list_t		fts_doc_list;	/*!< doc list to process */
 	fts_psort_common_t*	psort_common;	/*!< ptr to all psort info */
-	os_thread_t		thread_hdl;	/*!< thread handler */
 };
 
 /** Structure stores information from string tokenization operation */

=== modified file 'storage/innobase/log/log0recv.cc'
--- a/storage/innobase/log/log0recv.cc	revid:nirbhay.choubey@stripped
+++ b/storage/innobase/log/log0recv.cc	revid:kevin.lewis@stripped
@@ -168,7 +168,6 @@ UNIV_INTERN mysql_pfs_key_t	recv_writer_
 
 /** Flag indicating if recv_writer thread is active. */
 UNIV_INTERN bool		recv_writer_thread_active = false;
-UNIV_INTERN os_thread_t		recv_writer_thread_handle = 0;
 #endif /* !UNIV_HOTBACKUP */
 
 /* prototypes */
@@ -3271,9 +3270,7 @@ recv_recovery_from_checkpoint_start_func
 				/* Spawn the background thread to
 				flush dirty pages from the buffer
 				pools. */
-				recv_writer_thread_handle =
-					os_thread_create(
-					recv_writer_thread, 0, 0);
+				os_thread_create(recv_writer_thread, 0, 0);
 			} else {
 				/* Init the doublewrite buffer memory
 				 structure */
@@ -3443,12 +3440,6 @@ recv_recovery_from_checkpoint_finish(voi
 		}
 	}
 
-#ifdef __WIN__
-	if (recv_writer_thread_handle) {
-		CloseHandle(recv_writer_thread_handle);
-	}
-#endif /* __WIN__ */
-
 #ifndef UNIV_LOG_DEBUG
 	recv_sys_debug_free();
 #endif

=== modified file 'storage/innobase/os/os0thread.cc'
--- a/storage/innobase/os/os0thread.cc	revid:nirbhay.choubey@stripped
+++ b/storage/innobase/os/os0thread.cc	revid:kevin.lewis@stripped
@@ -100,11 +100,13 @@ os_thread_get_curr_id(void)
 
 /****************************************************************//**
 Creates a new thread of execution. The execution starts from
-the function given. The start function takes a void* parameter
-and returns an ulint.
-@return	handle to the thread */
+the function given.
+NOTE: We count the number of threads in os_thread_exit(). A created
+thread should always use that to exit so thatthe thread count will be
+decremented.
+We do not return an error code because if there is one, we crash here. */
 UNIV_INTERN
-os_thread_t
+void
 os_thread_create_func(
 /*==================*/
 	os_thread_func_t	func,		/*!< in: pointer to function
@@ -115,28 +117,43 @@ os_thread_create_func(
 						thread, or NULL */
 {
 #ifdef __WIN__
-	os_thread_t	thread;
+	HANDLE		handle;
 	DWORD		win_thread_id;
 
 	os_mutex_enter(os_sync_mutex);
 	os_thread_count++;
 	os_mutex_exit(os_sync_mutex);
 
-	thread = CreateThread(NULL,	/* no security attributes */
+	handle = CreateThread(NULL,	/* no security attributes */
 			      0,	/* default size stack */
 			      func,
 			      arg,
 			      0,	/* thread runs immediately */
 			      &win_thread_id);
 
-	if (thread_id) {
-		*thread_id = win_thread_id;
+	if (handle) {
+		if (thread_id) {
+			*thread_id = win_thread_id;
+		}
+
+		/* Innodb does not use the handle outside this thread.
+		It only uses the thread_id.  So close the handle now.
+		The thread object is held open by the thread until it
+		exits. */
+		CloseHandle(handle);
+	} else {
+		/* If we cannot start a new thread, life has no meaning. */
+		fprintf(stderr,
+			"InnoDB: Error: CreateThread returned %d\n",
+			GetLastError());
+		ut_ad(0);
+		exit(1);
 	}
 
-	return(thread);
-#else
+#else /* __WIN__ else */
+
 	int		ret;
-	os_thread_t	pthread;
+	os_thread_id_t	pthread_id;
 	pthread_attr_t	attr;
 
 #ifndef UNIV_HPUX10
@@ -156,6 +173,7 @@ os_thread_create_func(
 		fprintf(stderr,
 			"InnoDB: Error: pthread_attr_setstacksize"
 			" returned %d\n", ret);
+		ut_ad(0);
 		exit(1);
 	}
 #endif
@@ -164,25 +182,21 @@ os_thread_create_func(
 	os_mutex_exit(os_sync_mutex);
 
 #ifdef UNIV_HPUX10
-	ret = pthread_create(&pthread, pthread_attr_default, func, arg);
+	ret = pthread_create(&pthread_id, pthread_attr_default, func, arg);
 #else
-	ret = pthread_create(&pthread, &attr, func, arg);
+	ret = pthread_create(&pthread_id, &attr, func, arg);
 #endif
 	if (ret) {
 		fprintf(stderr,
 			"InnoDB: Error: pthread_create returned %d\n", ret);
+		ut_ad(0);
 		exit(1);
 	}
 
 #ifndef UNIV_HPUX10
 	pthread_attr_destroy(&attr);
 #endif
-	if (thread_id) {
-		*thread_id = pthread;
-	}
-
-	return(pthread);
-#endif
+#endif /* not __WIN__ */
 }
 
 /*****************************************************************//**
@@ -208,7 +222,14 @@ os_thread_exit(
 	os_mutex_exit(os_sync_mutex);
 
 #ifdef __WIN__
+# ifndef __cplusplus
+	/* "ExitThread is the preferred method of exiting a thread in C code
+	However, in C++ code, the thread is exited before any destructors
+	can be called or any other automatic cleanup can be performed.
+	Therefore, in C++ code, you should return from your thread function."
+	(msdn.microsoft.com)  */
 	ExitThread((DWORD) exit_value);
+# endif
 #else
 	pthread_detach(pthread_self());
 	pthread_exit(exit_value);

=== modified file 'storage/innobase/row/row0ftsort.cc'
--- a/storage/innobase/row/row0ftsort.cc	revid:nirbhay.choubey@stripped
+++ b/storage/innobase/row/row0ftsort.cc	revid:kevin.lewis@stripped
@@ -845,10 +845,6 @@ func_exit:
 	os_event_set(psort_info->psort_common->sort_event);
 	psort_info->child_status = FTS_CHILD_EXITING;
 
-#ifdef __WIN__
-	CloseHandle(psort_info->thread_hdl);
-#endif /*__WIN__ */
-
 	os_thread_exit(NULL);
 
 	OS_THREAD_DUMMY_RETURN;
@@ -867,9 +863,9 @@ row_fts_start_psort(
 
 	for (i = 0; i < fts_sort_pll_degree; i++) {
 		psort_info[i].psort_id = i;
-		psort_info[i].thread_hdl = os_thread_create(
-			fts_parallel_tokenization,
-			(void*) &psort_info[i], &thd_id);
+		os_thread_create(fts_parallel_tokenization,
+				 (void*) &psort_info[i],
+				 &thd_id);
 	}
 }
 
@@ -897,10 +893,6 @@ fts_parallel_merge(
 	os_event_set(psort_info->psort_common->merge_event);
 	psort_info->child_status = FTS_CHILD_EXITING;
 
-#ifdef __WIN__
-	CloseHandle(psort_info->thread_hdl);
-#endif /*__WIN__ */
-
 	os_thread_exit(NULL);
 
 	OS_THREAD_DUMMY_RETURN;
@@ -922,8 +914,9 @@ row_fts_start_parallel_merge(
 		merge_info[i].psort_id = i;
 		merge_info[i].child_status = 0;
 
-		merge_info[i].thread_hdl = os_thread_create(
-			fts_parallel_merge, (void*) &merge_info[i], &thd_id);
+		os_thread_create(fts_parallel_merge,
+				 (void*) &merge_info[i],
+				 &thd_id);
 	}
 }
 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (kevin.lewis:5304 to 5305) kevin.lewis11 Jan