Reduce redundant setting of thread name. (mono/mono#16253)

Maintain a counter and only set if the counter has changed.
This cannot be fully thread safe, so is not.
Any thread can set any other thread name at any time.
You could be atomic to do better, and loop to do even better,
but the thread name could still be changed again right after the loop exists.

The previous code was also not reliable in this way, since again the name
could be changed right away.

mono/mono#16248

Commit migrated from https://github.com/mono/mono/commit/63e1828f7e1c9694f34fd7189df6abc5cde1f192

diff --git a/src/mono/configure.ac b/src/mono/configure.ac
index 559efba..f48ee90 100644 (file)
--- a/src/mono/configure.ac
+++ b/src/mono/configure.ac
@@ -62,7 +62,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=2cfaeda3-94a9-44e5-8fcb-51979a3398c9
+MONO_CORLIB_VERSION=ea86fac2-6390-4597-9db9-6d3c05adcd06
 
 #
 # 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 ce7cb9c..2a12b7b 100644 (file)
--- a/src/mono/mono/metadata/object-internals.h
+++ b/src/mono/mono/metadata/object-internals.h
@@ -574,6 +574,7 @@ struct _MonoInternalThread {
        MonoThreadHandle *handle;
        gpointer native_handle;
        gunichar2  *name;
+       volatile gsize name_generation;
        guint32     name_len;
        guint32     state;      /* must be accessed while longlived->synch_cs is locked */
        MonoException *abort_exc;

diff --git a/src/mono/mono/metadata/threadpool.c b/src/mono/mono/metadata/threadpool.c
index acbd6ce..2cf8323 100644 (file)
--- a/src/mono/mono/metadata/threadpool.c
+++ b/src/mono/mono/metadata/threadpool.c
@@ -325,6 +325,8 @@ worker_callback (void)
 
        previous_tpdomain = NULL;
 
+       gsize name_generation = ~thread->name_generation;
+
        while (!mono_runtime_is_shutting_down ()) {
                gboolean retire = FALSE;
 
@@ -352,10 +354,17 @@ worker_callback (void)
 
                domains_unlock ();
 
-               MonoString *thread_name = mono_string_new_checked (mono_get_root_domain (), "Thread Pool Worker", error);
-               mono_error_assert_ok (error);
-               mono_thread_set_name_internal (thread, thread_name, MonoSetThreadNameFlag_Reset, error);
-               mono_error_assert_ok (error);
+               // 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) {
+                       MonoString *thread_name = mono_string_new_checked (mono_get_root_domain (), "Thread Pool Worker", error);
+                       mono_error_assert_ok (error);
+                       name_generation = mono_thread_set_name_internal (thread, thread_name, MonoSetThreadNameFlag_Reset, error);
+                       mono_error_assert_ok (error);
+               }
 
                mono_thread_clear_and_set_state (thread,
                        (MonoThreadState)~ThreadState_Background,

diff --git a/src/mono/mono/metadata/threads-types.h b/src/mono/mono/metadata/threads-types.h
index 716b4c0..a29c038 100644 (file)
--- a/src/mono/mono/metadata/threads-types.h
+++ b/src/mono/mono/metadata/threads-types.h
@@ -350,7 +350,7 @@ typedef enum {
 G_ENUM_FUNCTIONS (MonoSetThreadNameFlags)
 
 MONO_PROFILER_API
-void
+gsize
 mono_thread_set_name_internal (MonoInternalThread *thread,
                               MonoString *name,
                               MonoSetThreadNameFlags flags, MonoError *error);

diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c
index dc0233b..90f7f46 100644 (file)
--- a/src/mono/mono/metadata/threads.c
+++ b/src/mono/mono/metadata/threads.c
@@ -1857,23 +1857,32 @@ ves_icall_System_Threading_Thread_GetName_internal (MonoInternalThreadHandle thr
 }
 #endif
 
-void
+gsize
 mono_thread_set_name_internal (MonoInternalThread *this_obj,
                               MonoString *name,
                               MonoSetThreadNameFlags flags, MonoError *error)
 {
        MonoNativeThreadId tid = 0;
 
+       // A counter to optimize redundant sets.
+       // It is not exactly thread safe but no use of it could be.
+       gsize name_generation;
+
        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) {
                UNLOCK_THREAD (this_obj);
                
                mono_error_set_invalid_operation (error, "%s", "Thread.Name can only be set once.");
-               return;
+               return name_generation;
        }
+
+       name_generation = ++this_obj->name_generation;
+
        if (this_obj->name) {
                g_free (this_obj->name);
                this_obj->name_len = 0;
@@ -1895,13 +1904,16 @@ mono_thread_set_name_internal (MonoInternalThread *this_obj,
 
        if (this_obj->name && tid) {
                char *tname = mono_string_to_utf8_checked_internal (name, error);
-               return_if_nok (error);
-               MONO_PROFILER_RAISE (thread_name, ((uintptr_t)tid, tname));
-               mono_native_thread_set_name (tid, tname);
-               mono_free (tname);
+               if (is_ok (error)) {
+                       MONO_PROFILER_RAISE (thread_name, ((uintptr_t)tid, tname));
+                       mono_native_thread_set_name (tid, tname);
+                       mono_free (tname);
+               }
        }
 
        mono_thread_set_name_windows (this_obj->native_handle, name ? mono_string_chars_internal (name) : NULL);
+
+       return name_generation;
 }
 
 void 

diff --git a/src/mono/netcore/System.Private.CoreLib/src/System.Threading/Thread.cs b/src/mono/netcore/System.Private.CoreLib/src/System.Threading/Thread.cs
index 41bce71..cedea65 100644 (file)
--- a/src/mono/netcore/System.Private.CoreLib/src/System.Threading/Thread.cs
+++ b/src/mono/netcore/System.Private.CoreLib/src/System.Threading/Thread.cs
@@ -22,6 +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_len;
                private ThreadState state;
                private object abort_exc;