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

Fix Crash When Enabling and Disabling ETW with Old Callbacks #21086

Merged

Conversation

ivberg
Copy link
Contributor

@ivberg ivberg commented Jun 18, 2024

Description

Under certain conditions with enabling & disabling ETW continuously, we got a crash report.
Allows ETW callbacks to be de-registered upon class destructor.
Related to #20537

Motivation and Context

Fixes crash

Callstack

We see it crash in
[0x0] onnxruntime!<lambda_967a738fca8512372f170fcaf2d094d4>::operator()+0x34 0x12941ff570 0x7ffa994f0a04

[0x1] onnxruntime!std::_Func_class<void,_GUID const *,unsigned long,unsigned char,unsigned __int64,unsigned __int64,_EVENT_FILTER_DESCRIPTOR *,void *>::operator()+0x54 0x12941ff7b0 0x7ffa994f0d64

[0x2] onnxruntime!onnxruntime::logging::EtwRegistrationManager::InvokeCallbacks+0xcc 0x12941ff7b0 0x7ffa994f0d64

[0x3] onnxruntime!onnxruntime::logging::EtwRegistrationManager::ORT_TL_EtwEnableCallback+0x94 0x12941ff860 0x7ffa98d19628

and seems to us that the this pointer captured in
etwRegistrationManager.RegisterInternalCallback(
[&etwRegistrationManager, this](
...
is no longer valid when the callback is called.

@ivberg ivberg requested a review from skottmckay June 19, 2024 23:07
@microsoft microsoft deleted a comment from azure-pipelines bot Jun 20, 2024
@ivberg

This comment was marked as outdated.

This comment was marked as outdated.

@ivberg ivberg merged commit 55f7f9d into main Jun 20, 2024
100 checks passed
@ivberg ivberg deleted the user/ivberg/FixCrash_When_EnablingDisabling_ETW_OldCallback branch June 20, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants