diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5e62f39..f43e7f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,17 +106,44 @@ jobs: restore-keys: | ${{ runner.os }}-ccache-node- + - name: Install OpenSSL (Windows) + if: ${{ matrix.platform == 'win32' }} + shell: pwsh + run: | + choco install openssl.light -y --no-progress + # Shining Light keeps the x64/MD import libs under lib/VC/x64/MD, + # which CMake's FindOpenSSL does not search (and it ignores the + # OPENSSL_*_LIBRARY env vars), so stage a standard layout in a + # space-free dir and point OPENSSL_ROOT_DIR at it. + New-Item -ItemType Directory -Force -Path D:\openssl\lib | Out-Null + Copy-Item -Recurse 'C:\Program Files\OpenSSL\include' D:\openssl\ + Copy-Item 'C:\Program Files\OpenSSL\lib\VC\x64\MD\libcrypto.lib' D:\openssl\lib\ + Copy-Item 'C:\Program Files\OpenSSL\lib\VC\x64\MD\libssl.lib' D:\openssl\lib\ + "OPENSSL_ROOT_DIR=D:\openssl" >> $env:GITHUB_ENV + # Runtime DLLs (libcrypto/libssl-3-x64.dll) for the test step. + 'C:\Program Files\OpenSSL\bin' >> $env:GITHUB_PATH + Get-ChildItem D:\openssl -Recurse | ForEach-Object { $_.FullName } + + - name: Set up MSVC environment (Windows) + if: ${{ matrix.platform == 'win32' }} + uses: ilammy/msvc-dev-cmd@v1 + with: + arch: x64 + - name: Build Node.js native module (Windows) if: ${{ matrix.platform == 'win32' }} shell: cmd env: EXTRA_CMAKE_FLAGS: -DLBUG_NODEJS_ENABLE_TEST_EXPORTS=ON + # windows-2025 images ship mingw64 in PATH, which CMake would + # otherwise prefer over MSVC (breaking ws2_32 linking and ar). + # (cl itself comes from msvc-dev-cmd above via vswhere.) + CC: cl + CXX: cl run: | powershell.exe -Command "Add-MpPreference -ExclusionPath '${{ github.workspace }}'" - call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvars64.bat" - ccache -s - make GEN=Ninja nodejs ccache -s + make GEN=Ninja nodejs && ccache -s - name: Build Node.js native module (non-Windows) if: ${{ matrix.platform != 'win32' }} diff --git a/src_js/connection.js b/src_js/connection.js index 61f6ef4..8bd6be5 100644 --- a/src_js/connection.js +++ b/src_js/connection.js @@ -41,18 +41,23 @@ class Connection { if (this._isClosed) { throw new Error("Connection is closed."); } - if (!this._isInitialized) { - if (!this._initPromise) { + if (this._isInitialized) { + return; + } + if (!this._initPromise) { + // Cache the promise synchronously (no await before assignment) so + // concurrent init() calls share a single initialization instead of + // each starting their own (which crashed the process, see #24). + this._initPromise = (async () => { + const database = await this._database._getDatabase(); if (!this._connection) { - const database = await this._database._getDatabase(); this._connection = new LbugNative.NodeConnection(database); } - this._initPromise = new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { this._connection.initAsync((err) => { if (err) { reject(err); } else { - this._isInitialized = true; if (this._numThreads) { this._connection.setMaxNumThreadForExec(this._numThreads); } @@ -63,8 +68,21 @@ class Connection { } }); }); + this._isInitialized = true; + })(); + } + const initPromise = this._initPromise; + try { + await initPromise; + } catch (err) { + // Allow retry after a failed initialization (clear only if no + // newer init has started in the meantime). + if (this._initPromise === initPromise) { + this._initPromise = null; } - await this._initPromise; + throw err; + } + if (this._initPromise === initPromise) { this._initPromise = null; } } diff --git a/test/test_concurrency.js b/test/test_concurrency.js index 8b70776..3952ecc 100644 --- a/test/test_concurrency.js +++ b/test/test_concurrency.js @@ -74,3 +74,55 @@ describe("Concurrent query execution across multiple connections", function () { assert.isFalse(results[4][0]["a.isStudent"]); }); }); + +describe("Concurrent initialization on a fresh connection (issue #24)", function () { + it("should handle concurrent queries as the very first use without explicit init", async function () { + // Regression test for https://github.com/LadybugDB/ladybug-nodejs/issues/24. + // Each iteration uses a fresh in-memory database + connection and fires + // two queries concurrently as the first operation. Before the fix, the + // lazy-init race in Connection.init() crashed the process (SIGSEGV). + // bufferPoolSize 64 MB, maxDBSize 1 GB (the default maxDBSize asks for + // an 8 TB mmap, which is too large for some environments). + for (let i = 0; i < 50; i++) { + const memDb = new lbug.Database( + ":memory:", + 64 * 1024 * 1024, + true, + false, + 1024 * 1024 * 1024 + ); + const freshConn = new lbug.Connection(memDb); + const queryResults = await Promise.all([ + freshConn.query("RETURN 1 AS x"), + freshConn.query("RETURN 2 AS x"), + ]); + const rows = await Promise.all( + queryResults.map((queryResult) => queryResult.getAll()) + ); + assert.deepEqual(rows[0], [{ x: 1 }]); + assert.deepEqual(rows[1], [{ x: 2 }]); + for (const queryResult of queryResults) { + queryResult.close(); + } + await freshConn.close(); + await memDb.close(); + } + }); + + it("should handle concurrent init() calls on a fresh connection", async function () { + const memDb = new lbug.Database( + ":memory:", + 64 * 1024 * 1024, + true, + false, + 1024 * 1024 * 1024 + ); + const freshConn = new lbug.Connection(memDb); + await Promise.all([freshConn.init(), freshConn.init()]); + const queryResult = await freshConn.query("RETURN 1 AS x"); + assert.deepEqual(await queryResult.getAll(), [{ x: 1 }]); + queryResult.close(); + await freshConn.close(); + await memDb.close(); + }); +});