Skip to content

Commit c4624d0

Browse files
authored
[EventPipe] Block eventpipe delete provider for pending callbacks (#106156)
* [EventPipe] Block ep_delete_provider for pending callbacks * Free callbacks complete event * Address feedback
1 parent 5ebf090 commit c4624d0

File tree

2 files changed

+21
-17
lines changed

2 files changed

+21
-17
lines changed

src/native/eventpipe/ep-provider.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ provider_free (EventPipeProvider * provider)
493493

494494
dn_list_custom_free (provider->event_list, event_free_func);
495495

496+
ep_rt_wait_event_free (&provider->callbacks_complete_event);
496497
ep_rt_utf16_string_free (provider->provider_name_utf16);
497498
ep_rt_utf8_string_free (provider->provider_name);
498499
ep_rt_object_free (provider);

src/native/eventpipe/ep.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,9 @@ disable_holding_lock (
644644
ep_session_free (session);
645645

646646
// Providers can't be deleted during tracing because they may be needed when serializing the file.
647-
config_delete_deferred_providers(ep_config_get ());
647+
// Allow delete deferred providers to accumulate to mitigate potential use-after-free should
648+
// another EventPipe session hold a reference to a provider set for deferred deletion.
649+
// config_delete_deferred_providers(ep_config_get ());
648650
}
649651

650652
ep_requires_lock_held ();
@@ -1319,22 +1321,18 @@ ep_delete_provider (EventPipeProvider *provider)
13191321
// where we hold a provider after tracing has been disabled.
13201322
bool wait_for_provider_callbacks_completion = false;
13211323
EP_LOCK_ENTER (section1)
1322-
if (enabled ()) {
1323-
// Save the provider until the end of the tracing session.
1324-
ep_provider_set_delete_deferred (provider, true);
1325-
1326-
// The callback func must be previously set to null,
1327-
// otherwise callbacks might never stop coming.
1328-
EP_ASSERT (provider->callback_func == NULL);
1329-
1330-
// Calling ep_delete_provider within a Callback will result in a deadlock
1331-
// as deleting the provider with an active tracing session will block
1332-
// until all of the provider's callbacks are completed.
1333-
if (provider->callbacks_pending > 0)
1334-
wait_for_provider_callbacks_completion = true;
1335-
} else {
1336-
config_delete_provider (ep_config_get (), provider);
1337-
}
1324+
// Save the provider until the end of the tracing session.
1325+
ep_provider_set_delete_deferred (provider, true);
1326+
1327+
// The callback func must be set to null,
1328+
// otherwise callbacks might never stop coming.
1329+
EP_ASSERT (provider->callback_func == NULL);
1330+
1331+
// Calling ep_delete_provider within a Callback will result in a deadlock
1332+
// as deleting the provider with an active tracing session will block
1333+
// until all of the provider's callbacks are completed.
1334+
if (provider->callbacks_pending > 0)
1335+
wait_for_provider_callbacks_completion = true;
13381336
EP_LOCK_EXIT (section1)
13391337

13401338
// Block provider deletion until all pending callbacks are completed.
@@ -1344,6 +1342,11 @@ ep_delete_provider (EventPipeProvider *provider)
13441342
if (wait_for_provider_callbacks_completion)
13451343
ep_rt_wait_event_wait (&provider->callbacks_complete_event, EP_INFINITE_WAIT, false);
13461344

1345+
EP_LOCK_ENTER (section2)
1346+
if (!enabled ())
1347+
config_delete_provider (ep_config_get (), provider);
1348+
EP_LOCK_EXIT (section2)
1349+
13471350
ep_on_exit:
13481351
ep_requires_lock_not_held ();
13491352
return;

0 commit comments

Comments
 (0)