Skip to content

Commit af2a388

Browse files
adrianlizarragaguschmue
authored andcommitted
[QNN EP] Fix segfault when unregistering HTP shared memory handles (#23402)
### Description - Fixes segfault when the function that cleans up HTP memory handles uses an invalid Logger. - Fixes unit test that compares output from QNN EP with exact float values. QNN HTP runs float32 models with float16 precision, so need to use a tolerance in the comparison. ### Motivation and Context Fixes issues with using QNN HTP memory sharing on Windows ARM64. This is also needed to test HTP shared memory with #23120.
1 parent f3a4e99 commit af2a388

File tree

3 files changed

+28
-9
lines changed

3 files changed

+28
-9
lines changed

onnxruntime/core/providers/qnn/builder/qnn_context_mem_handle_manager.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,17 @@ Status QnnContextMemHandleManager::GetOrRegister(void* shared_memory_address, co
8686

8787
LOGS(logger_, VERBOSE) << "Registered QNN mem handle. mem_handle: " << raw_mem_handle;
8888

89-
const auto unregister_mem_handle = [this](Qnn_MemHandle_t raw_mem_handle) {
90-
LOGS(logger_, VERBOSE) << "Unregistering QNN mem handle. mem_handle: " << raw_mem_handle;
91-
92-
const auto unregister_result = qnn_interface_.memDeRegister(&raw_mem_handle, 1);
89+
// NOTE: Must use the default ORT logger inside this lambda. Don't capture this->logger_ because it may be deleted
90+
// by the time we need to unregister all memory handles. This happens when this->logger_ is a session logger:
91+
// ~InferenceSession() -> ~Logger() -> ~QnnExecutionProvider() -> ~QnnBackendManager() ->
92+
// ~QnnContextMemHandleManager() -> unregister_mem_handle() segfault
93+
const auto unregister_mem_handle = [&qnn_interface = this->qnn_interface_](Qnn_MemHandle_t raw_mem_handle) {
94+
LOGS_DEFAULT(VERBOSE) << "Unregistering QNN mem handle. mem_handle: " << raw_mem_handle;
95+
96+
const auto unregister_result = qnn_interface.memDeRegister(&raw_mem_handle, 1);
9397
if (unregister_result != QNN_SUCCESS) {
94-
LOGS(logger_, ERROR) << "qnn_interface.memDeRegister() failed: "
95-
<< utils::GetVerboseQnnErrorMessage(qnn_interface_, unregister_result);
98+
LOGS_DEFAULT(ERROR) << "qnn_interface.memDeRegister() failed: "
99+
<< utils::GetVerboseQnnErrorMessage(qnn_interface, unregister_result);
96100
}
97101
};
98102

onnxruntime/test/providers/qnn/qnn_basic_test.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,12 @@ TEST_F(QnnHTPBackendTests, UseHtpSharedMemoryAllocatorForInputs) {
11131113
qnn_ep = QnnExecutionProviderWithOptions(provider_options);
11141114
} catch (const OnnxRuntimeException& e) {
11151115
// handle particular exception that indicates that the libcdsprpc.so / dll can't be loaded
1116+
// NOTE: To run this on a local Windows ARM64 device, you need to copy libcdsprpc.dll to the build directory:
1117+
// - Open File Explorer
1118+
// - Go to C:/Windows/System32/DriverStore/FileRepository/
1119+
// - Search for a folder that begins with qcnspmcdm8380.inf_arm64_ and open it
1120+
// - Copy the libcdsprpc.dll into the build/[PATH CONTAINING onnxruntime.dll] directory of the application.
1121+
// TODO(adrianlizarraga): Update CMake build for unittests to automatically copy libcdsprpc.dll into build directory
11161122
#if defined(_WIN32)
11171123
constexpr const char* expected_error_message = "Failed to load libcdsprpc.dll";
11181124
#else

onnxruntime/test/shared_lib/test_inference.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,6 +1960,12 @@ static bool CreateSessionWithQnnEpAndQnnHtpSharedMemoryAllocator(PATH_TYPE model
19601960
return true;
19611961
} catch (const Ort::Exception& e) {
19621962
// handle particular exception that indicates that the libcdsprpc.so / dll can't be loaded
1963+
// NOTE: To run this on a local Windows ARM64 device, you need to copy libcdsprpc.dll to the build directory:
1964+
// - Open File Explorer
1965+
// - Go to C:/Windows/System32/DriverStore/FileRepository/
1966+
// - Search for a folder that begins with qcnspmcdm8380.inf_arm64_ and open it
1967+
// - Copy the libcdsprpc.dll into the build/[PATH CONTAINING onnxruntime.dll] directory of the application.
1968+
// TODO(adrianlizarraga): Update CMake build for unittests to automatically copy libcdsprpc.dll into build directory
19631969
std::string_view error_message = e.what();
19641970

19651971
#if defined(_WIN32)
@@ -2275,8 +2281,11 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
22752281
Ort::Value bound_x = Ort::Value::CreateTensor(info_qnn_htp_shared, reinterpret_cast<float*>(input_data.get()), x_values.size(),
22762282
x_shape.data(), x_shape.size());
22772283

2284+
// Setup expected output (y) from model. Note that QNN EP runs float32 operators as float16,
2285+
// so the output will not be exactly equal.
22782286
const std::array<int64_t, 2> expected_y_shape = {3, 2};
22792287
const std::array<float, 3 * 2> expected_y = {1.0f, 4.0f, 9.0f, 16.0f, 25.0f, 36.0f};
2288+
constexpr float y_max_abs_err = 1e-5f;
22802289
auto output_data = qnn_htp_shared_allocator.GetAllocation(expected_y.size() * sizeof(float));
22812290
ASSERT_NE(output_data.get(), nullptr);
22822291

@@ -2293,7 +2302,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
22932302
// Check the values against the bound raw memory
22942303
{
22952304
gsl::span y{reinterpret_cast<const float*>(output_data.get()), expected_y.size()};
2296-
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
2305+
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
22972306
}
22982307

22992308
// Now compare values via GetOutputValues
@@ -2308,7 +2317,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
23082317
ASSERT_EQ(expected_y.size(), count);
23092318

23102319
gsl::span y{Y_value.GetTensorData<float>(), count};
2311-
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
2320+
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
23122321
}
23132322

23142323
{
@@ -2336,7 +2345,7 @@ TEST(CApiTest, io_binding_qnn_htp_shared) {
23362345
ASSERT_EQ(expected_y.size(), count);
23372346

23382347
gsl::span y{Y_value.GetTensorData<float>(), count};
2339-
ASSERT_TRUE(std::equal(std::begin(y), std::end(y), std::begin(expected_y)));
2348+
EXPECT_THAT(expected_y, ::testing::Pointwise(::testing::FloatNear(y_max_abs_err), y));
23402349
}
23412350

23422351
// Clean up

0 commit comments

Comments
 (0)