Set thread pool thread name just based on string pointer check. (mono…

…/mono#16637)

* Set thread pool thread name just based on string pointer check.

When the generation-based solution went in, there were not
yet constant thread names, so a pointer based approach could fail
due to reuse. We can do slightly simpler now.

* Remove the generation field which is no longer needed.
Remove the duplicated knowledge of how to set a constant and put
it behind a funny sounding but perhaps reasonable flag.
Revise corlib version.


Commit migrated from mono/mono@2129ac6

src/mono/configure.ac

@@ -63,7 +63,7 @@ MONO_VERSION_BUILD=`echo $VERSION | cut -d . -f 3`
 # This line is parsed by tools besides autoconf, such as msvc/mono.winconfig.targets.
 # It should remain in the format they expect.
 #
-MONO_CORLIB_VERSION=3bf09a16-d684-401b-ae3c-2015596b0194
+MONO_CORLIB_VERSION=EEEBAE82-DB4F-4A2C-977F-7C83DE83E547
 
 #
 # Put a quoted #define in config.h.

src/mono/mono/metadata/object-internals.h

@@ -564,8 +564,7 @@ struct _MonoThreadInfo;
 
 typedef struct MonoThreadName {
 	char* volatile chars;      // null check outside of lock
-	gsize volatile generation; // read outside of lock
-	gint32 free;
+	gint32 free; // bool
 	gint32 length;
 } MonoThreadName;
 

src/mono/mono/metadata/threadpool.c

@@ -293,10 +293,13 @@ try_invoke_perform_wait_callback (MonoObject** exc, MonoError *error)
 	HANDLE_FUNCTION_RETURN_VAL (res);
 }
 
-static gsize
-set_thread_name (MonoInternalThread *thread)
+static void
+mono_threadpool_set_thread_name (MonoInternalThread *thread)
 {
-	return mono_thread_set_name_constant_ignore_error (thread, "Thread Pool Worker", MonoSetThreadNameFlag_Reset);
+	mono_thread_set_name_constant_ignore_error (
+		thread,
+		"Thread Pool Worker",
+		MonoSetThreadNameFlag_Reset | MonoSetThreadNameFlag_RepeatedlyButOptimized);
 }
 
 static void
@@ -334,10 +337,8 @@ worker_callback (void)
 	 */
 	mono_defaults.threadpool_perform_wait_callback_method->save_lmf = TRUE;
 
-	gsize name_generation = thread->name.generation;
 	/* Set the name if this is the first call to worker_callback on this thread */
-	if (name_generation == 0)
-	   name_generation = set_thread_name (thread);
+	mono_threadpool_set_thread_name (thread);
 
 	domains_lock ();
 
@@ -370,13 +371,7 @@ worker_callback (void)
 
 		domains_unlock ();
 
-		// Any thread can set any other thread name at any time.
-		// So this is unavoidably racy.
-		// This only partly fights against that -- i.e. not atomic and not a loop.
-		// It is reliable against the thread setting its own name, and somewhat
-		// reliable against other threads setting this thread's name.
-		if (name_generation != thread->name.generation)
-			name_generation = set_thread_name (thread);
+		mono_threadpool_set_thread_name (thread);
 
 		mono_thread_clear_and_set_state (thread,
 			(MonoThreadState)~ThreadState_Background,
@@ -404,8 +399,7 @@ worker_callback (void)
 		mono_thread_pop_appdomain_ref ();
 
 		/* Reset name after every callback */
-		if (name_generation != thread->name.generation)
-			name_generation = set_thread_name (thread);
+		mono_threadpool_set_thread_name (thread);
 
 		domains_lock ();
 

src/mono/mono/metadata/threads-types.h

@@ -342,12 +342,13 @@ typedef enum {
     MonoSetThreadNameFlag_Permanent = 0x0001,
     MonoSetThreadNameFlag_Reset     = 0x0002,
     MonoSetThreadNameFlag_Constant  = 0x0004,
+    MonoSetThreadNameFlag_RepeatedlyButOptimized = 0x0008,
 } MonoSetThreadNameFlags;
 
 G_ENUM_FUNCTIONS (MonoSetThreadNameFlags)
 
 MONO_PROFILER_API
-gsize
+void
 mono_thread_set_name (MonoInternalThread *thread,
 		      const char* name8, size_t name8_length, const gunichar2* name16,
 		      MonoSetThreadNameFlags flags, MonoError *error);

src/mono/mono/metadata/threads.c

@@ -1751,11 +1751,7 @@ void
 mono_thread_name_cleanup (MonoThreadName* name)
 {
 	MonoThreadName const old_name = *name;
-	// Do not reset generation.
-	name->chars = 0;
-	name->length = 0;
-	name->free = 0;
-	//memset (name, 0, sizeof (*name));
+	memset (name, 0, sizeof (*name));
 	if (old_name.free)
 		g_free (old_name.chars);
 }
@@ -1939,16 +1935,24 @@ ves_icall_System_Threading_Thread_GetName_internal (MonoInternalThreadHandle thr
 //  - name16 only used on Windows.
 //  - name8 is either constant, or g_free'able -- this function always takes ownership and never copies.
 //
-gsize
+void
 mono_thread_set_name (MonoInternalThread *this_obj,
 		      const char* name8, size_t name8_length, const gunichar2* name16,
 		      MonoSetThreadNameFlags flags, MonoError *error)
 {
-	MonoNativeThreadId tid = 0;
+	// A special case for the threadpool worker.
+	// It sets the name repeatedly, in case it has changed, per-spec,
+	// and if not done carefully, this can be a performance problem.
+	//
+	// This is racy but ok. The name will always be valid (or absent).
+	// In an unusual race, the name might briefly not revert to what the spec requires.
+	//
+	if ((flags & MonoSetThreadNameFlag_RepeatedlyButOptimized) && name8 == this_obj->name.chars) {
+		// Length is presumed to match.
+		return;
+	}
 
-	// A counter to optimize redundant sets.
-	// It is not exactly thread safe but no use of it could be.
-	gsize name_generation;
+	MonoNativeThreadId tid = 0;
 
 	const gboolean constant = !!(flags & MonoSetThreadNameFlag_Constant);
 
@@ -1958,8 +1962,6 @@ mono_thread_set_name (MonoInternalThread *this_obj,
 
 	LOCK_THREAD (this_obj);
 
-	name_generation = this_obj->name.generation;
-
 	if (flags & MonoSetThreadNameFlag_Reset) {
 		this_obj->flags &= ~MONO_THREAD_FLAG_NAME_SET;
 	} else if (this_obj->flags & MONO_THREAD_FLAG_NAME_SET) {
@@ -1970,11 +1972,9 @@ mono_thread_set_name (MonoInternalThread *this_obj,
 
 		if (!constant)
 			g_free ((char*)name8);
-		return name_generation;
+		return;
 	}
 
-	name_generation = ++this_obj->name.generation;
-
 	mono_thread_name_cleanup (&this_obj->name);
 
 	if (name8) {
@@ -1998,9 +1998,6 @@ mono_thread_set_name (MonoInternalThread *this_obj,
 	mono_thread_set_name_windows (this_obj->native_handle, name16);
 
 	mono_free (0); // FIXME keep mono-publib.c in use and its functions exported
-
-	return name_generation;
-
 }
 
 void 

src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs

@@ -22,8 +22,7 @@ partial class Thread
 		IntPtr native_handle; // used only on Win32
 		/* accessed only from unmanaged code */
 		private IntPtr name;
-		private IntPtr name_generation;
-		private int name_free;
+		private int name_free; // bool
 		private int name_length;
 		private ThreadState state;
 		private object abort_exc;