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 https://github.com/mono/mono/commit/2129ac6c282560e3039a38de9ebbebcdfab6f149

diff --git a/src/mono/configure.ac b/src/mono/configure.ac
index 00b7529..2f920d6 100644 (file)
--- a/src/mono/configure.ac
+++ b/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.

diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h
index c4b5670..4e43783 100644 (file)
--- a/src/mono/mono/metadata/object-internals.h
+++ b/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;
 

diff --git a/src/mono/mono/metadata/threadpool.c b/src/mono/mono/metadata/threadpool.c
index a1d3e49..ecbe81a 100644 (file)
--- a/src/mono/mono/metadata/threadpool.c
+++ b/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 ();
 

diff --git a/src/mono/mono/metadata/threads-types.h b/src/mono/mono/metadata/threads-types.h
index 9f4ddd8..dc06fb5 100644 (file)
--- a/src/mono/mono/metadata/threads-types.h
+++ b/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);

diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c
index 0cdcab6..9a2e5d7 100644 (file)
--- a/src/mono/mono/metadata/threads.c
+++ b/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 

diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
index 102e79a..4ebd11b 100644 (file)
--- a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
+++ b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
@@ -22,8 +22,7 @@ namespace System.Threading
                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;