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 in QNN EP - ResetQnnLogLevel #22456

Merged
merged 5 commits into from
Oct 23, 2024

Conversation

ivberg
Copy link
Contributor

@ivberg ivberg commented Oct 15, 2024

Description

Fix crash with extra checks ResetQnnLogLevel.

From the dump it looks like during ETW callbacks, while the provider is stopping, we attempt to reset the QNN log level.
While the QNN BackEndMgr (this) is alive logger_ is not valid

Motivation and Context

ORT should not crash

@ivberg ivberg requested a review from HectorSVC October 15, 2024 23:28
@ivberg ivberg requested a review from HectorSVC October 17, 2024 16:53
@ivberg ivberg force-pushed the user/ivberg/ResetQnnLogLevel_CrashFish branch from e8ecb53 to 840a671 Compare October 22, 2024 17:08
@HectorSVC
Copy link
Contributor

HectorSVC commented Oct 22, 2024

Is it possible to create some UTs to cover ETW feature?
for example, have a thread with session creation/destruction in a loop for 1000 times. and another thread keeps changing the ETW log level.

@ivberg
Copy link
Contributor Author

ivberg commented Oct 22, 2024

Is it possible to create some UTs to cover ETW feature? for example, have a thread with session creation/destruction in a loop for 1000 times. and another thread keeps changing the ETW log level.

I do like the idea but need to see if possible? Ok if we do in another PR as timing need to get a fix in place ASAP

@HectorSVC HectorSVC merged commit 0028d3f into main Oct 23, 2024
91 checks passed
@HectorSVC HectorSVC deleted the user/ivberg/ResetQnnLogLevel_CrashFish branch October 23, 2024 03:45
@sophies927 sophies927 added release:1.20.0 triage:approved Approved for cherrypicks for release labels Oct 23, 2024
apsonawane pushed a commit that referenced this pull request Oct 28, 2024
### Description
Fix crash with extra checks ResetQnnLogLevel. 

From the dump it looks like during ETW callbacks, while the provider is stopping, we attempt to reset the QNN log level.
While the QNN BackEndMgr (this) is alive logger_ is not valid

### Motivation and Context
ORT should not crash
apsonawane pushed a commit that referenced this pull request Oct 28, 2024
### Description
Fix crash with extra checks ResetQnnLogLevel. 

From the dump it looks like during ETW callbacks, while the provider is stopping, we attempt to reset the QNN log level.
While the QNN BackEndMgr (this) is alive logger_ is not valid

### Motivation and Context
ORT should not crash
apsonawane pushed a commit that referenced this pull request Oct 29, 2024
### Description
Fix crash with extra checks ResetQnnLogLevel. 

From the dump it looks like during ETW callbacks, while the provider is stopping, we attempt to reset the QNN log level.
While the QNN BackEndMgr (this) is alive logger_ is not valid

### Motivation and Context
ORT should not crash
ishwar-raut1 pushed a commit to ishwar-raut1/onnxruntime that referenced this pull request Nov 19, 2024
### Description
Fix crash with extra checks ResetQnnLogLevel. 

From the dump it looks like during ETW callbacks, while the provider is stopping, we attempt to reset the QNN log level.
While the QNN BackEndMgr (this) is alive logger_ is not valid

### Motivation and Context
ORT should not crash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:1.20.0 triage:approved Approved for cherrypicks for release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants