From 24a80c058a86155c4cf0a2ef3c8edcb0efab3c15 Mon Sep 17 00:00:00 2001 From: John Salem Date: Wed, 4 May 2022 17:00:05 +0000 Subject: [PATCH] Apply changes from #63356 --- .../vm/eventing/eventpipe/ep-rt-coreclr.cpp | 2 +- .../vm/eventing/eventpipe/ep-rt-coreclr.h | 37 ++++++++++++++----- src/mono/mono/eventpipe/ep-rt-mono.h | 8 ++++ src/native/eventpipe/ep-rt.h | 4 ++ 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp index e09b6b1600745..6826030ff5480 100644 --- a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp +++ b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp @@ -12,7 +12,7 @@ CrstStatic _ep_rt_coreclr_config_lock; thread_local EventPipeCoreCLRThreadHolderTLS EventPipeCoreCLRThreadHolderTLS::g_threadHolderTLS; -ep_char8_t *_ep_rt_coreclr_diagnostics_cmd_line; +ep_char8_t *volatile _ep_rt_coreclr_diagnostics_cmd_line; #ifndef TARGET_UNIX uint32_t *_ep_rt_coreclr_proc_group_offsets; diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h index 9744e84a89b69..f0c5d544373c4 100644 --- a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h @@ -1216,6 +1216,15 @@ ep_rt_atomic_compare_exchange_size_t (volatile size_t *target, size_t expected, return static_cast(InterlockedCompareExchangeT (target, value, expected)); } +static +inline +ep_char8_t * +ep_rt_atomic_compare_exchange_utf8_string (ep_char8_t *volatile *target, ep_char8_t *expected, ep_char8_t *value) +{ + STATIC_CONTRACT_NOTHROW; + return static_cast(InterlockedCompareExchangeT (target, value, expected)); +} + /* * EventPipe. */ @@ -2664,19 +2673,29 @@ ep_rt_diagnostics_command_line_get (void) STATIC_CONTRACT_NOTHROW; // In coreclr, this value can change over time, specifically before vs after suspension in diagnostics server. - // The host initalizes the runtime in two phases, init and exec assembly. On non-Windows platforms the commandline returned by the runtime + // The host initializes the runtime in two phases, init and exec assembly. On non-Windows platforms the commandline returned by the runtime // is different during each phase. We suspend during init where the runtime has populated the commandline with a // mock value (the full path of the executing assembly) and the actual value isn't populated till the exec assembly phase. - // On Windows this does not apply as the value is retrieved directly from the OS any time it is requested. + // On Windows this does not apply as the value is retrieved directly from the OS any time it is requested. // As a result, we cannot actually cache this value. We need to return the _current_ value. // This function needs to handle freeing the string in order to make it consistent with Mono's version. - // To that end, we'll "cache" it here so we free the previous string when we get it again. - extern ep_char8_t *_ep_rt_coreclr_diagnostics_cmd_line; - - if (_ep_rt_coreclr_diagnostics_cmd_line) - ep_rt_utf8_string_free(_ep_rt_coreclr_diagnostics_cmd_line); - - _ep_rt_coreclr_diagnostics_cmd_line = ep_rt_utf16_to_utf8_string (reinterpret_cast(GetCommandLineForDiagnostics ()), -1); + // There is a rare chance this may be called on multiple threads, so we attempt to always return the newest value + // and conservatively leak the old value if it changed. This is extremely rare and should only leak 1 string. + extern ep_char8_t *volatile _ep_rt_coreclr_diagnostics_cmd_line; + + ep_char8_t *old_cmd_line = _ep_rt_coreclr_diagnostics_cmd_line; + ep_char8_t *new_cmd_line = ep_rt_utf16_to_utf8_string (reinterpret_cast(GetCommandLineForDiagnostics ()), -1); + if (old_cmd_line && ep_rt_utf8_string_compare (old_cmd_line, new_cmd_line) == 0) { + // same as old, so free the new one + ep_rt_utf8_string_free (new_cmd_line); + } else { + // attempt an update, and give up if you lose the race + if (ep_rt_atomic_compare_exchange_utf8_string (&_ep_rt_coreclr_diagnostics_cmd_line, old_cmd_line, new_cmd_line) != old_cmd_line) { + ep_rt_utf8_string_free (new_cmd_line); + } + // NOTE: If there was a value we purposefully leak it since it may still be in use. + // This leak is *small* (length of the command line) and bounded (should only happen once) + } return _ep_rt_coreclr_diagnostics_cmd_line; } diff --git a/src/mono/mono/eventpipe/ep-rt-mono.h b/src/mono/mono/eventpipe/ep-rt-mono.h index dbba8b0249937..6c954f1be8486 100644 --- a/src/mono/mono/eventpipe/ep-rt-mono.h +++ b/src/mono/mono/eventpipe/ep-rt-mono.h @@ -603,6 +603,14 @@ ep_rt_atomic_compare_exchange_size_t (volatile size_t *target, size_t expected, #endif } +static +inline +ep_char8_t * +ep_rt_atomic_compare_exchange_utf8_string (ep_char8_t *volatile *target, ep_char8_t *expected, ep_char8_t *value) +{ + return (ep_char8_t *)mono_atomic_cas_ptr ((volatile gpointer *)target, (gpointer)value, (gpointer)expected); +} + /* * EventPipe. */ diff --git a/src/native/eventpipe/ep-rt.h b/src/native/eventpipe/ep-rt.h index 4fc22632c6c17..8684e155a5d98 100644 --- a/src/native/eventpipe/ep-rt.h +++ b/src/native/eventpipe/ep-rt.h @@ -182,6 +182,10 @@ static size_t ep_rt_atomic_compare_exchange_size_t (volatile size_t *target, size_t expected, size_t value); +static +ep_char8_t * +eo_rt_atomic_compare_exchange_utf8_string (volatile ep_char8_t **target, ep_char8_t *expected, ep_char8_t *value); + /* * EventPipe. */