[QNN EP] Make sure everything gets cleaned up - #23275
Conversation
|
/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline |
|
/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, Windows x64 QNN CI Pipeline, Linux MIGraphX CI Pipeline, Big Models |
|
Azure Pipelines successfully started running 9 pipeline(s). |
|
/azp run ONNX Runtime React Native CI Pipeline, Linux Android Emulator QNN CI Pipeline, Windows GPU CUDA CI Pipeline, Windows GPU DML CI Pipeline, Windows GPU Doc Gen CI Pipeline |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
Azure Pipelines successfully started running 5 pipeline(s). |
adrianlizarraga
left a comment
adrianlizarraga
left a comment
There was a problem hiding this comment.
Minor adjustments to log levels
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
|
/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline |
|
/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, Windows x64 QNN CI Pipeline, Linux MIGraphX CI Pipeline, Big Models |
|
/azp run ONNX Runtime React Native CI Pipeline, Linux Android Emulator QNN CI Pipeline, Windows GPU CUDA CI Pipeline, Windows GPU DML CI Pipeline, Windows GPU Doc Gen CI Pipeline |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
Azure Pipelines successfully started running 5 pipeline(s). |
|
Azure Pipelines successfully started running 9 pipeline(s). |
|
/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline |
|
/azp run ONNX Runtime React Native CI Pipeline, Linux Android Emulator QNN CI Pipeline, Windows GPU CUDA CI Pipeline, Windows GPU DML CI Pipeline, Windows GPU Doc Gen CI Pipeline |
|
/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, Windows x64 QNN CI Pipeline, Linux MIGraphX CI Pipeline, Big Models |
|
Azure Pipelines successfully started running 5 pipeline(s). |
|
Azure Pipelines successfully started running 9 pipeline(s). |
|
Azure Pipelines successfully started running 6 pipeline(s). |
| auto result = ReleaseContext(); | ||
| if (Status::OK() != result) { | ||
| ORT_THROW("Failed to ReleaseContext."); | ||
| LOGS_DEFAULT(ERROR) << "Failed to ReleaseContext."; |
There was a problem hiding this comment.
can we also log the error message from the Status?
There was a problem hiding this comment.
let's have another PR for this for the urgency.
### Description Always make sure resources and callbacks are cleaned up ### Motivation and Context We've seen problems where the log callback isn't deregistered which can lead to crashes --------- Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
| backend_setup_completed_ = true; | ||
| } else { | ||
| LOGS_DEFAULT(WARNING) << "Failed to setup so cleaning up"; | ||
| ReleaseResources(); |
There was a problem hiding this comment.
ReleaseResources() first checks backend_setup_completed_ == true before continuing.
onnxruntime/onnxruntime/core/providers/qnn/builder/qnn_backend_manager.cc
Lines 1094 to 1096 in 3328eb3
as far as I can tell, the only place backend_setup_completed_ gets set to true is in the other branch. so would the call to ReleaseResources() not actually do anything here?
### Description Always make sure resources and callbacks are cleaned up ### Motivation and Context We've seen problems where the log callback isn't deregistered which can lead to crashes --------- Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
### Description Always make sure resources and callbacks are cleaned up ### Motivation and Context We've seen problems where the log callback isn't deregistered which can lead to crashes --------- Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
### Description Always make sure resources and callbacks are cleaned up ### Motivation and Context We've seen problems where the log callback isn't deregistered which can lead to crashes --------- Co-authored-by: Adrian Lizarraga <adrianlm2@gmail.com>
5 participants
Description
Always make sure resources and callbacks are cleaned up
Motivation and Context
We've seen problems where the log callback isn't deregistered which can lead to crashes