-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[node] Fix logging mutex crash at exit on macOS #26445
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| #include "ort_singleton_data.h" | ||
|
Check warning on line 4 in js/node/src/ort_singleton_data.cc
|
||
|
|
||
| OrtSingletonData::OrtObjects::OrtObjects(int log_level) | ||
| : env{OrtLoggingLevel(log_level), "onnxruntime-node"}, | ||
| default_run_options{} { | ||
| } | ||
|
|
||
| OrtSingletonData::OrtObjects& OrtSingletonData::GetOrCreateOrtObjects(int log_level) { | ||
| static OrtObjects ort_objects(log_level); | ||
| return ort_objects; | ||
| } | ||
|
|
||
| const Ort::Env& OrtSingletonData::Env() { | ||
| return GetOrCreateOrtObjects().env; | ||
| } | ||
|
|
||
| const Ort::RunOptions& OrtSingletonData::DefaultRunOptions() { | ||
| return GetOrCreateOrtObjects().default_run_options; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <napi.h> | ||
| #include "onnxruntime_cxx_api.h" | ||
|
Check warning on line 7 in js/node/src/ort_singleton_data.h
|
||
|
|
||
| /** | ||
| * The OrtSingletonData class is designed to manage the lifecycle of necessary singleton instance data, including: | ||
| * | ||
| * - The Ort::Env singleton instance. | ||
| * This is a global singleton that is shared across all InferenceSessionWrap instances. It is created when the first | ||
| * time `InferenceSession.initOrtOnce()` is called. | ||
| * | ||
| * - The Ort::RunOptions singleton instance. | ||
| * This is an empty default RunOptions instance. It is created once to allow reuse across all session inference runs. | ||
| * | ||
| * The OrtSingletonData class uses the "Meyers Singleton" pattern to ensure thread-safe lazy initialization, as well as | ||
| * proper destruction order at program exit. | ||
| */ | ||
| struct OrtSingletonData { | ||
| struct OrtObjects { | ||
| Ort::Env env; | ||
| Ort::RunOptions default_run_options; | ||
|
|
||
| private: | ||
| // The following pattern ensures that OrtObjects can only be created by OrtSingletonData | ||
| OrtObjects(int log_level); | ||
|
Check warning on line 29 in js/node/src/ort_singleton_data.h
|
||
| friend struct OrtSingletonData; | ||
| }; | ||
|
|
||
| static OrtObjects& GetOrCreateOrtObjects(int log_level = ORT_LOGGING_LEVEL_WARNING); | ||
|
|
||
| // Get the global Ort::Env | ||
| static const Ort::Env& Env(); | ||
|
|
||
| // Get the default Ort::RunOptions | ||
| static const Ort::RunOptions& DefaultRunOptions(); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Process Exit Tests | ||
|
|
||
| These tests verify that the ORT Node.js binding handles various process exit scenarios without crashing, specifically addressing the mutex crash issue reported in [#24579](https://github.com/microsoft/onnxruntime/issues/24579). | ||
|
|
||
| ## What This Tests | ||
|
|
||
| - **Normal process exit** - Verifies clean shutdown without mutex crashes | ||
| - **`process.exit()` calls** - Tests the primary crash scenario that was fixed | ||
| - **Uncaught exceptions** - Ensures crashes don't occur during unexpected exits | ||
| - **Session cleanup** - Tests both explicit `session.release()` and automatic cleanup | ||
| - **Stability** - Multiple runs to ensure consistent behavior | ||
|
|
||
| ## How It Works | ||
|
|
||
| Each test runs in a separate Node.js process to isolate the test environment. Tests use command-line flags to control behavior: | ||
|
|
||
| - `--process-exit`: Triggers `process.exit(0)` | ||
| - `--throw-exception`: Throws an uncaught exception | ||
| - `--release`: Calls `session.release()` before exit | ||
|
|
||
| ## Expected Result | ||
|
|
||
| All tests should pass without `mutex lock failed` or `std::__1::system_error` messages in stderr. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import { spawn } from 'child_process'; | ||
| import * as assert from 'assert'; | ||
| import * as path from 'path'; | ||
|
|
||
| describe('Standalone Process Tests', () => { | ||
| // Helper function to run test script in a separate process | ||
| const runTest = async (args: string[] = []): Promise<{ code: number; stdout: string; stderr: string }> => | ||
| new Promise((resolve, reject) => { | ||
| // Use the compiled main.js file from the lib directory | ||
| const testFile = path.join(__dirname, './main.js'); | ||
|
|
||
| const child = spawn('node', [testFile, ...args], { stdio: 'pipe' }); | ||
|
|
||
| let stdout = ''; | ||
| let stderr = ''; | ||
|
|
||
| child.stdout.on('data', (data) => (stdout += data.toString())); | ||
| child.stderr.on('data', (data) => (stderr += data.toString())); | ||
|
|
||
| child.on('close', (code) => { | ||
| resolve({ code: code || 0, stdout, stderr }); | ||
| }); | ||
|
|
||
| child.on('error', reject); | ||
| }); | ||
|
|
||
| // Helper function to check basic success criteria | ||
| const assertSuccess = (result: { code: number; stdout: string; stderr: string }) => { | ||
| assert.strictEqual(result.code, 0); | ||
| assert.ok(result.stdout.includes('SUCCESS: Inference completed')); | ||
| assert.ok(!result.stderr.includes('mutex lock failed')); | ||
| }; | ||
|
|
||
| // Helper function to check that no mutex crashes occurred | ||
| const assertNoMutexErrors = (stderr: string) => { | ||
| assert.ok(!stderr.includes('mutex lock failed')); | ||
| assert.ok(!stderr.includes('std::__1::system_error')); | ||
| }; | ||
|
|
||
| it('should handle normal process exit', async () => { | ||
| const result = await runTest([]); | ||
| assertSuccess(result); | ||
| }); | ||
|
|
||
| it('should handle process.exit() call', async () => { | ||
| const result = await runTest(['--process-exit']); | ||
| assertSuccess(result); | ||
| }); | ||
|
|
||
| it('should handle uncaught exceptions', async () => { | ||
| const result = await runTest(['--throw-exception']); | ||
|
|
||
| assert.notStrictEqual(result.code, 0); | ||
| assert.ok(result.stdout.includes('SUCCESS: Inference completed')); | ||
| assert.ok(result.stderr.includes('Test exception')); | ||
| assertNoMutexErrors(result.stderr); | ||
| }); | ||
|
|
||
| it('should handle multiple process exits consistently', async () => { | ||
| for (let i = 0; i < 3; i++) { | ||
| const result = await runTest(['--process-exit']); | ||
| assertSuccess(result); | ||
| } | ||
| }); | ||
|
|
||
| it('should handle session.release() before normal exit', async () => { | ||
| const result = await runTest(['--release']); | ||
| assertSuccess(result); | ||
| assert.ok(result.stdout.includes('Session released')); | ||
| }); | ||
|
|
||
| it('should handle session.release() before process.exit()', async () => { | ||
| const result = await runTest(['--release', '--process-exit']); | ||
| assertSuccess(result); | ||
| assert.ok(result.stdout.includes('Session released')); | ||
| }); | ||
|
|
||
| it('should handle no session.release() before process.exit()', async () => { | ||
| const result = await runTest(['--process-exit']); | ||
| assertSuccess(result); | ||
| assert.ok(result.stdout.includes('Session NOT released')); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import * as path from 'path'; | ||
| const ort = require(path.join(__dirname, '../../')); | ||
| import * as process from 'process'; | ||
|
|
||
| const modelData = | ||
| 'CAMSDGJhY2tlbmQtdGVzdDpiChEKAWEKAWISAWMiBk1hdE11bBIOdGVzdF9tYXRtdWxfMmRaEwoBYRIOCgwIARIICgIIAwoCCARaEwoBYhIOCgwIARIICgIIBAoCCANiEwoBYxIOCgwIARIICgIIAwoCCANCAhAJ'; | ||
| const shouldProcessExit = process.argv.includes('--process-exit'); | ||
| const shouldThrowException = process.argv.includes('--throw-exception'); | ||
| const shouldRelease = process.argv.includes('--release'); | ||
|
|
||
| async function main() { | ||
| try { | ||
| const modelBuffer = Buffer.from(modelData, 'base64'); | ||
| const session = await ort.InferenceSession.create(modelBuffer); | ||
|
|
||
| const dataA = Float32Array.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); | ||
| const dataB = Float32Array.from([10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120]); | ||
| const tensorA = new ort.Tensor('float32', dataA, [3, 4]); | ||
| const tensorB = new ort.Tensor('float32', dataB, [4, 3]); | ||
|
|
||
| const results = await session.run({ a: tensorA, b: tensorB }); | ||
| console.log('SUCCESS: Inference completed'); | ||
| console.log(`Result: ${results.c.data}`); | ||
|
|
||
| if (shouldRelease) { | ||
| await session.release(); | ||
| console.log('Session released'); | ||
| } else { | ||
| console.log('Session NOT released (testing cleanup behavior)'); | ||
| } | ||
|
|
||
| if (shouldThrowException) { | ||
| setTimeout(() => { | ||
| throw new Error('Test exception'); | ||
| }, 10); | ||
| return; | ||
| } | ||
|
|
||
| if (shouldProcessExit) { | ||
| process.exit(0); | ||
| } | ||
| } catch (e) { | ||
| console.error(`ERROR: ${e}`); | ||
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| void main(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.