Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mono]: Fix infrequent infinite loop on Mono EventPipe streaming thread. #72517

Merged
merged 3 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/mono/mono/eventpipe/ep-rt-config-mono.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
#define __EVENTPIPE_RT_CONFIG_MONO_H__

#define EP_THREAD_INCLUDE_ACTIVITY_ID
#define EP_RT_USE_CUSTOM_HASH_MAP_CALLBACKS

#endif /* __EVENTPIPE_RT_CONFIG_MONO_H__ */
27 changes: 24 additions & 3 deletions src/mono/mono/eventpipe/ep-rt-mono.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ prefix_name ## _rt_ ## type_name ## _ ## func_name
} \
static inline bool EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, queue_name, is_empty) (const queue_type *queue) { \
EP_ASSERT (queue != NULL); \
return g_queue_is_empty (queue->queue); \
return (g_queue_is_empty (queue->queue) == TRUE) ? true : false; \
} \
static inline bool EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, queue_name, is_valid) (const queue_type *queue) { return (queue != NULL && queue->queue != NULL); }

Expand Down Expand Up @@ -258,7 +258,7 @@ prefix_name ## _rt_ ## type_name ## _ ## func_name
EP_RT_DEFINE_ARRAY_REVERSE_ITERATOR_PREFIX(ep, array_name, array_type, iterator_type, item_type)

#define EP_RT_DEFINE_HASH_MAP_BASE_PREFIX(prefix_name, hash_map_name, hash_map_type, key_type, value_type) \
static inline void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, alloc) (hash_map_type *hash_map, uint32_t (*hash_callback)(const void *), bool (*eq_callback)(const void *, const void *), void (*key_free_callback)(void *), void (*value_free_callback)(void *)) { \
static inline void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, alloc) (hash_map_type *hash_map, ep_rt_hash_map_hash_callback_t hash_callback, ep_rt_hash_map_equal_callback_t eq_callback, void (*key_free_callback)(void *), void (*value_free_callback)(void *)) { \
EP_ASSERT (hash_map != NULL); \
EP_ASSERT (key_free_callback == NULL); \
hash_map->table = g_hash_table_new_full ((GHashFunc)hash_callback, (GEqualFunc)eq_callback, (GDestroyNotify)key_free_callback, (GDestroyNotify)value_free_callback); \
Expand All @@ -270,6 +270,7 @@ prefix_name ## _rt_ ## type_name ## _ ## func_name
} \
static inline bool EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, add) (hash_map_type *hash_map, key_type key, value_type value) { \
EP_ASSERT (hash_map != NULL); \
EP_ASSERT (!g_hash_table_lookup_extended (hash_map->table, (gconstpointer)key, NULL, NULL)); \
g_hash_table_insert (hash_map->table, (gpointer)key, ((gpointer)(gsize)value)); \
return true; \
} \
Expand All @@ -280,7 +281,7 @@ prefix_name ## _rt_ ## type_name ## _ ## func_name
static inline bool EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, lookup) (const hash_map_type *hash_map, const key_type key, value_type *value) { \
EP_ASSERT (hash_map != NULL && value != NULL); \
gpointer _value = NULL; \
bool result = g_hash_table_lookup_extended (hash_map->table, (gconstpointer)key, NULL, &_value); \
bool result = (g_hash_table_lookup_extended (hash_map->table, (gconstpointer)key, NULL, &_value) == TRUE) ? true : false; \
*value = ((value_type)(gsize)_value); \
return result; \
} \
Expand Down Expand Up @@ -380,6 +381,8 @@ extern int64_t ep_rt_mono_system_timestamp_get (void);
extern void ep_rt_mono_os_environment_get_utf16 (ep_rt_env_array_utf16_t *env_array);
extern MonoNativeTlsKey _ep_rt_mono_thread_holder_tls_id;
extern EventPipeThread * ep_rt_mono_thread_get_or_create (void);
extern uint32_t ep_stack_hash_key_hash (const void *key);
extern bool ep_stack_hash_key_equal (const void *key1, const void *key2);

static
inline
Expand Down Expand Up @@ -877,6 +880,24 @@ EP_RT_DEFINE_HASH_MAP_REMOVE(metadata_labels_hash, ep_rt_metadata_labels_hash_ma
EP_RT_DEFINE_HASH_MAP(stack_hash, ep_rt_stack_hash_map_t, StackHashKey *, StackHashEntry *)
EP_RT_DEFINE_HASH_MAP_ITERATOR(stack_hash, ep_rt_stack_hash_map_t, ep_rt_stack_hash_map_iterator_t, StackHashKey *, StackHashEntry *)

#ifdef EP_RT_USE_CUSTOM_HASH_MAP_CALLBACKS
static
inline
guint
ep_rt_stack_hash_key_hash (gconstpointer key)
{
return (guint)ep_stack_hash_key_hash (key);
}

static
inline
gboolean
ep_rt_stack_hash_key_equal (gconstpointer key1, gconstpointer key2)
{
return !!ep_stack_hash_key_equal (key1, key2);
}
#endif

/*
* EventPipeProvider.
*/
Expand Down
5 changes: 5 additions & 0 deletions src/mono/mono/eventpipe/ep-rt-types-mono.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ struct _rt_mono_array_iterator_internal_t {
int32_t index;
};

#ifdef EP_RT_USE_CUSTOM_HASH_MAP_CALLBACKS
typedef GHashFunc ep_rt_hash_map_hash_callback_t;
typedef GEqualFunc ep_rt_hash_map_equal_callback_t;
#endif

struct _rt_mono_table_internal_t {
GHashTable *table;
};
Expand Down
2 changes: 1 addition & 1 deletion src/native/eventpipe/ep-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ ep_file_alloc (
ep_rt_metadata_labels_hash_alloc (&instance->metadata_ids, NULL, NULL, NULL, NULL);
ep_raise_error_if_nok (ep_rt_metadata_labels_hash_is_valid (&instance->metadata_ids));

ep_rt_stack_hash_alloc (&instance->stack_hash, ep_stack_hash_key_hash, ep_stack_hash_key_equal, NULL, stack_hash_value_free_func);
ep_rt_stack_hash_alloc (&instance->stack_hash, ep_rt_stack_hash_key_hash, ep_rt_stack_hash_key_equal, NULL, stack_hash_value_free_func);
ep_raise_error_if_nok (ep_rt_stack_hash_is_valid (&instance->stack_hash));

// Start at 0 - The value is always incremented prior to use, so the first ID will be 1.
Expand Down
12 changes: 11 additions & 1 deletion src/native/eventpipe/ep-rt.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,13 @@ prefix_name ## _rt_ ## type_name ## _ ## func_name

#define EP_RT_DEFINE_ARRAY_REVERSE_ITERATOR ep_rt_redefine

#ifndef EP_RT_USE_CUSTOM_HASH_MAP_CALLBACKS
typedef uint32_t (*ep_rt_hash_map_hash_callback_t)(const void *);
typedef bool (*ep_rt_hash_map_equal_callback_t)(const void *, const void *);
#endif

#define EP_RT_DECLARE_HASH_MAP_BASE_PREFIX(prefix_name, hash_map_name, hash_map_type, key_type, value_type) \
static void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, alloc) (hash_map_type *hash_map, uint32_t (*hash_callback)(const void *), bool (*eq_callback)(const void *, const void *), void (*key_free_callback)(void *), void (*value_free_callback)(void *)); \
static void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, alloc) (hash_map_type *hash_map, ep_rt_hash_map_hash_callback_t hash_callback, ep_rt_hash_map_equal_callback_t eq_callback, void (*key_free_callback)(void *), void (*value_free_callback)(void *)); \
static void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, free) (hash_map_type *hash_map); \
static bool EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, add) (hash_map_type *hash_map, key_type key, value_type value); \
static void EP_RT_BUILD_TYPE_FUNC_NAME(prefix_name, hash_map_name, remove_all) (hash_map_type *hash_map); \
Expand Down Expand Up @@ -349,6 +354,11 @@ EP_RT_DECLARE_HASH_MAP_REMOVE(metadata_labels_hash, ep_rt_metadata_labels_hash_m
EP_RT_DECLARE_HASH_MAP(stack_hash, ep_rt_stack_hash_map_t, StackHashKey *, StackHashEntry *)
EP_RT_DECLARE_HASH_MAP_ITERATOR(stack_hash, ep_rt_stack_hash_map_t, ep_rt_stack_hash_map_iterator_t, StackHashKey *, StackHashEntry *)

#ifndef EP_RT_USE_CUSTOM_HASH_MAP_CALLBACKS
#define ep_rt_stack_hash_key_hash ep_stack_hash_key_hash
#define ep_rt_stack_hash_key_equal ep_stack_hash_key_equal
#endif

/*
* EventPipeProvider.
*/
Expand Down
12 changes: 6 additions & 6 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2084,12 +2084,6 @@
<ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/gcdump/gcdump/**">
<Issue>needs triage</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/providervalidation/**">
<Issue>https://github.com/dotnet/runtime/issues/59296</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/rundownvalidation/**">
<Issue>https://github.com/dotnet/runtime/issues/54801</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/DllImportAttribute/DllImportPath/**">
<Issue>needs triage</Issue>
</ExcludeList>
Expand Down Expand Up @@ -3674,6 +3668,12 @@
<ExcludeList Include = "$(XunitTestBinBase)/tracing/runtimeeventsource/nativeruntimeeventsource/*">
<Issue>WASM doesn't support diagnostics tracing</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/providervalidation/**">
<Issue>WASM doesn't support diagnostics tracing</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/rundownvalidation/**">
<Issue>WASM doesn't support diagnostics tracing</Issue>
</ExcludeList>
</ItemGroup>

<ItemGroup Condition=" $(TargetOS) == 'Android' " >
Expand Down