From 8ee483d7e0df8057cad7f6883400c7d16c1dfee0 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 11:50:33 +0200 Subject: [PATCH 1/7] add an asan CI Test --- .github/workflows/test-dev.yml | 19 +++++++++++++++++++ test/napi-leaks-suppression.txt | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/napi-leaks-suppression.txt diff --git a/.github/workflows/test-dev.yml b/.github/workflows/test-dev.yml index 8481117..af50896 100644 --- a/.github/workflows/test-dev.yml +++ b/.github/workflows/test-dev.yml @@ -229,3 +229,22 @@ jobs: directory: ${{ github.workspace }}/coverage token: ${{ secrets.CODECOV_TOKEN }} verbose: true + + + asan: + runs-on: ubuntu-22.04 + + steps: + - uses: actions/checkout@v4 + - name: Use Node.js 18.x + uses: actions/setup-node@v4 + with: + node-version: 18.x + - run: npm install --ignore-scripts + - run: npx @mapbox/node-pre-gyp configure --debug --enable_asan + - run: npx @mapbox/node-pre-gyp build -j max + - run: npm test + env: + MOCHA_REPEAT: 10 + LD_PRELOAD: /usr/lib/x86_64-linux-gnu/libasan.so.6 + LSAN_OPTIONS: suppressions=${{ github.workspace }}/test/napi-leaks-suppression.txt diff --git a/test/napi-leaks-suppression.txt b/test/napi-leaks-suppression.txt new file mode 100644 index 0000000..e02b362 --- /dev/null +++ b/test/napi-leaks-suppression.txt @@ -0,0 +1,29 @@ +# All leaks listed here are non-repeat offenders +# (ie they happen only a fixed number of times per process execution) + +# Known leaks in Node-API +leak:napi_module_register +leak:napi_register_module_v1 + +# Known leaks in the Node.js runtime +leak:node::builtins::BuiltinLoader::LoadBuiltinSource +leak:ProcessWrap::OnExit +leak:StringBytes::Encode +leak:Realm::ExecuteBootstrapper + +# Known leaks in V8 +leak:Heap_GenerationalBarrierSlow +leak:Scavenger::ScavengePage +leak:Scavenger::Process +leak:CompactionSpace::Expand +leak:Heap::IterateRoots +leak:Heap::PerformGarbageCollection +leak:Heap::InsertIntoRememberedSetFromCode +leak:Heap::SetUpSpaces +leak:Heap::PerformGarbageCollection +leak:PagedSpace::RefillLabMain +leak:OldLargeObjectSpace::AllocateRaw +leak:BaselineBatchCompiler::EnqueueFunction +leak:Compiler::FinalizeTurbofanCompilationJob +leak:v8::internal::Factory::CodeBuilder::Build +leak:v8::internal::MemoryChunk::RegisterObjectWithInvalidatedSlots From 3825617167490d7ae0b9fb5e00d11365f1e16617 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 12:03:33 +0200 Subject: [PATCH 2/7] switch to the mocha that repeats --- package-lock.json | 152 +++++++++++++++++++++++++++++++--------------- package.json | 2 +- 2 files changed, 105 insertions(+), 49 deletions(-) diff --git a/package-lock.json b/package-lock.json index baa5c3a..c85de0a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34,7 +34,7 @@ "eslint-plugin-array-func": "^5.0.1", "eslint-plugin-mocha": "^10.4.1", "eslint-plugin-prefer-arrow": "^1.2.3", - "mocha": "^10.1.0", + "mocha": "github:mmomtchev/mocha#mmom", "node-gyp": "^10.0.1", "ts-node": "^10.9.2", "tsconfig-paths": "^4.1.2", @@ -1626,10 +1626,11 @@ } }, "node_modules/ansi-colors": { - "version": "4.1.3", - "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-4.1.3.tgz", - "integrity": "sha512-/6w/C21Pm1A7aZitlI5Ni/2J6FFQN8i1Cvz3kHABAAbw93v/NlvKdVOqz7CCWz/3iv/JplRSEEZ83XION15ovw==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-4.1.1.tgz", + "integrity": "sha512-JoX0apGbHaUJBNl6yF+p6JAFYZ666/hhCGKN5t9QFjbJQKUU/g8MNbFDbvfrgKXvI1QpZplPOnwIo99lX/AAmA==", "dev": true, + "license": "MIT", "engines": { "node": ">=6" } @@ -5218,31 +5219,32 @@ } }, "node_modules/mocha": { - "version": "10.7.3", - "resolved": "https://registry.npmjs.org/mocha/-/mocha-10.7.3.tgz", - "integrity": "sha512-uQWxAu44wwiACGqjbPYmjo7Lg8sFrS3dQe7PP2FQI+woptP4vZXSMcfMyFL/e1yFEeEpV4RtyTpZROOKmxis+A==", - "dev": true, - "dependencies": { - "ansi-colors": "^4.1.3", - "browser-stdout": "^1.3.1", - "chokidar": "^3.5.3", - "debug": "^4.3.5", - "diff": "^5.2.0", - "escape-string-regexp": "^4.0.0", - "find-up": "^5.0.0", - "glob": "^8.1.0", - "he": "^1.2.0", - "js-yaml": "^4.1.0", - "log-symbols": "^4.1.0", - "minimatch": "^5.1.6", - "ms": "^2.1.3", - "serialize-javascript": "^6.0.2", - "strip-json-comments": "^3.1.1", - "supports-color": "^8.1.1", - "workerpool": "^6.5.1", - "yargs": "^16.2.0", - "yargs-parser": "^20.2.9", - "yargs-unparser": "^2.0.0" + "version": "10.2.0", + "resolved": "git+ssh://git@github.com/mmomtchev/mocha.git#04116c50b287d54b034c5eaf020595f74782c145", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-colors": "4.1.1", + "browser-stdout": "1.3.1", + "chokidar": "3.5.3", + "debug": "4.3.4", + "diff": "5.0.0", + "escape-string-regexp": "4.0.0", + "find-up": "5.0.0", + "glob": "8.1.0", + "he": "1.2.0", + "js-yaml": "4.1.0", + "log-symbols": "4.1.0", + "minimatch": "5.0.1", + "ms": "2.1.3", + "nanoid": "3.3.3", + "serialize-javascript": "6.0.0", + "strip-json-comments": "3.1.1", + "supports-color": "8.1.1", + "workerpool": "6.2.1", + "yargs": "16.2.0", + "yargs-parser": "20.2.4", + "yargs-unparser": "2.0.0" }, "bin": { "_mocha": "bin/_mocha", @@ -5257,6 +5259,7 @@ "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", "dev": true, + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -5272,11 +5275,47 @@ "wrap-ansi": "^7.0.0" } }, + "node_modules/mocha/node_modules/debug": { + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "ms": "2.1.2" + }, + "engines": { + "node": ">=6.0" + }, + "peerDependenciesMeta": { + "supports-color": { + "optional": true + } + } + }, + "node_modules/mocha/node_modules/debug/node_modules/ms": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", + "dev": true, + "license": "MIT" + }, + "node_modules/mocha/node_modules/diff": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-5.0.0.tgz", + "integrity": "sha512-/VTCrvm5Z0JGty/BWHljh+BAiw3IK+2j87NGMu8Nwc/f48WoDAC395uomO9ZD117ZOBaHmkX1oyLvkVM/aIT3w==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/mocha/node_modules/minimatch": { - "version": "5.1.6", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.1.6.tgz", - "integrity": "sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.0.1.tgz", + "integrity": "sha512-nLDxIFRyhDblz3qMuq+SoRZED4+miJ/G+tdDrjkkkRnjAsBexeGpgjLEQ0blJy7rHhR2b93rhQY4SvyWu9v03g==", "dev": true, + "license": "ISC", "dependencies": { "brace-expansion": "^2.0.1" }, @@ -5337,6 +5376,16 @@ "node": ">=10" } }, + "node_modules/mocha/node_modules/yargs-parser": { + "version": "20.2.4", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.4.tgz", + "integrity": "sha512-WOkpgNhPTlE73h4VFAFsOnomJVaovO8VqLDzy5saChRBFQFBoMYirowyW+Q9HB4HFF4Z7VZTiG3iSzJJA29yRA==", + "dev": true, + "license": "ISC", + "engines": { + "node": ">=10" + } + }, "node_modules/mri": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/mri/-/mri-1.2.0.tgz", @@ -5351,6 +5400,19 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, + "node_modules/nanoid": { + "version": "3.3.3", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.3.tgz", + "integrity": "sha512-p1sjXuopFs0xg+fPASzQ28agW1oHD7xDsd9Xkf3T15H3c/cifrFHVwrh74PdoklAPi+i7MdRsE47vm2r6JoB+w==", + "dev": true, + "license": "MIT", + "bin": { + "nanoid": "bin/nanoid.cjs" + }, + "engines": { + "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" + } + }, "node_modules/natural-compare": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/natural-compare/-/natural-compare-1.4.0.tgz", @@ -5985,6 +6047,7 @@ "resolved": "https://registry.npmjs.org/randombytes/-/randombytes-2.1.0.tgz", "integrity": "sha512-vYl3iOX+4CKUWuxGi9Ukhie6fsqXqS9FE2Zaic4tNFD2N2QQaXOMFbuKK4QmDHC0JO6B1Zp41J0LpT0oR68amQ==", "dev": true, + "license": "MIT", "dependencies": { "safe-buffer": "^5.1.0" } @@ -6455,10 +6518,11 @@ "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==" }, "node_modules/serialize-javascript": { - "version": "6.0.2", - "resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.2.tgz", - "integrity": "sha512-Saa1xPByTTq2gdeFZYLLo+RFE35NHZkAbqZeWNd3BpzppeVisAqpDjcp8dyf6uIvEqJRd46jemmyA4iFIeVk8g==", + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.0.tgz", + "integrity": "sha512-Qr3TosvguFt8ePWqsvRfrKyQXIiW+nGbYpy8XK24NQHE83caxWt+mIymTT19DGFbNWNLfEwsrkSmN64lVWB9ag==", "dev": true, + "license": "BSD-3-Clause", "dependencies": { "randombytes": "^2.1.0" } @@ -7407,10 +7471,11 @@ } }, "node_modules/workerpool": { - "version": "6.5.1", - "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.5.1.tgz", - "integrity": "sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA==", - "dev": true + "version": "6.2.1", + "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.1.tgz", + "integrity": "sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw==", + "dev": true, + "license": "Apache-2.0" }, "node_modules/wrap-ansi": { "version": "6.2.0", @@ -7482,15 +7547,6 @@ "node": ">=12" } }, - "node_modules/yargs-parser": { - "version": "20.2.9", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.9.tgz", - "integrity": "sha512-y11nGElTIV+CT3Zv9t7VKl+Q3hTQoT9a1Qzezhhl6Rp21gJ/IVTW7Z3y9EWXhuUBC2Shnf+DX0antecpAwSP8w==", - "dev": true, - "engines": { - "node": ">=10" - } - }, "node_modules/yargs-unparser": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/yargs-unparser/-/yargs-unparser-2.0.0.tgz", diff --git a/package.json b/package.json index c6fc586..5a1777c 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "eslint-plugin-array-func": "^5.0.1", "eslint-plugin-mocha": "^10.4.1", "eslint-plugin-prefer-arrow": "^1.2.3", - "mocha": "^10.1.0", + "mocha": "github:mmomtchev/mocha#mmom", "node-gyp": "^10.0.1", "ts-node": "^10.9.2", "tsconfig-paths": "^4.1.2", From 42ab4d9ad406c035f38f6489f2ce2696f4a3f137 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 16:16:25 +0200 Subject: [PATCH 3/7] fix the leak --- .github/workflows/test-dev.yml | 3 +-- .vscode/launch.json | 8 ++++++-- test/napi-leaks-suppression.txt | 3 +++ test/types.test.ts | 15 ++++++++++----- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test-dev.yml b/.github/workflows/test-dev.yml index af50896..4069d40 100644 --- a/.github/workflows/test-dev.yml +++ b/.github/workflows/test-dev.yml @@ -243,8 +243,7 @@ jobs: - run: npm install --ignore-scripts - run: npx @mapbox/node-pre-gyp configure --debug --enable_asan - run: npx @mapbox/node-pre-gyp build -j max - - run: npm test + - run: npm test -- --repeats 10 env: - MOCHA_REPEAT: 10 LD_PRELOAD: /usr/lib/x86_64-linux-gnu/libasan.so.6 LSAN_OPTIONS: suppressions=${{ github.workspace }}/test/napi-leaks-suppression.txt diff --git a/.vscode/launch.json b/.vscode/launch.json index d6aca18..c94c783 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -70,7 +70,11 @@ "runtimeArgs": [ "--expose-gc", ], - "program": "${workspaceFolder}/node_modules/mocha/lib/cli/cli.js" + "program": "${workspaceFolder}/node_modules/mocha/lib/cli/cli.js", + "args": [ + "--timeout", + "0" + ] }, { "name": "Launch benchmarks (gdb)", @@ -128,4 +132,4 @@ "program": "${file}" } ] -} \ No newline at end of file +} diff --git a/test/napi-leaks-suppression.txt b/test/napi-leaks-suppression.txt index e02b362..c728b31 100644 --- a/test/napi-leaks-suppression.txt +++ b/test/napi-leaks-suppression.txt @@ -1,5 +1,6 @@ # All leaks listed here are non-repeat offenders # (ie they happen only a fixed number of times per process execution) +# The list is maintained for Node.js 18 # Known leaks in Node-API leak:napi_module_register @@ -27,3 +28,5 @@ leak:BaselineBatchCompiler::EnqueueFunction leak:Compiler::FinalizeTurbofanCompilationJob leak:v8::internal::Factory::CodeBuilder::Build leak:v8::internal::MemoryChunk::RegisterObjectWithInvalidatedSlots +leak:v8::ScriptCompiler +leak:v8::internal::Genesis::Genesis diff --git a/test/types.test.ts b/test/types.test.ts index 72a88e3..5edac48 100644 --- a/test/types.test.ts +++ b/test/types.test.ts @@ -746,11 +746,16 @@ describe('types', () => { assert.isTrue(fn.callable); assert.equal(fn.type, 'pymport.js_function'); - const py_catch = pymport('python_helpers').get('dont_catch_exception'); - - assert.throws(() => { - py_catch.call(fn); - }, /Python exception: JS exception/); + const py_dont_catch = pymport('python_helpers').get('dont_catch_exception'); + + // V8 seems to have a problem with garbage collecting functions + // referenced in closures + // https://github.com/mmomtchev/pymport/issues/283 + try { + py_dont_catch.call(fn); + } catch (e: any) { + assert.match(e.toString(), /Python exception: JS exception/); + } }); it('unsupported arguments', () => { From 9def87d534133512eee44e20d078c5b4c3c2cecb Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 16:19:06 +0200 Subject: [PATCH 4/7] install the python requirements --- .github/workflows/test-dev.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-dev.yml b/.github/workflows/test-dev.yml index 4069d40..0660a27 100644 --- a/.github/workflows/test-dev.yml +++ b/.github/workflows/test-dev.yml @@ -215,7 +215,7 @@ jobs: node-version: 18.x - run: npm install --ignore-scripts - run: npx node-pre-gyp configure --debug --enable_coverage - - run: npx node-pre-gyp build + - run: npx node-pre-gyp build -j max - run: python3 -m pip install --upgrade pip - run: pip3 install -r test/requirements.txt - run: npx c8 npm test @@ -243,6 +243,8 @@ jobs: - run: npm install --ignore-scripts - run: npx @mapbox/node-pre-gyp configure --debug --enable_asan - run: npx @mapbox/node-pre-gyp build -j max + - run: python3 -m pip install --upgrade pip + - run: pip3 install -r test/requirements.txt - run: npm test -- --repeats 10 env: LD_PRELOAD: /usr/lib/x86_64-linux-gnu/libasan.so.6 From b0463303e0c2cb40f2881ef6ec55954ff8b31839 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 16:20:54 +0200 Subject: [PATCH 5/7] test that the exception has actually been raised --- test/types.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/types.test.ts b/test/types.test.ts index 5edac48..457e04f 100644 --- a/test/types.test.ts +++ b/test/types.test.ts @@ -751,11 +751,14 @@ describe('types', () => { // V8 seems to have a problem with garbage collecting functions // referenced in closures // https://github.com/mmomtchev/pymport/issues/283 + let failed = false; try { py_dont_catch.call(fn); } catch (e: any) { assert.match(e.toString(), /Python exception: JS exception/); + failed = true; } + assert.isTrue(failed); }); it('unsupported arguments', () => { From 21ceb13eff7649f56948f938ec16fdb3fdfad4a6 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 16:25:35 +0200 Subject: [PATCH 6/7] ignore Python for now --- test/napi-leaks-suppression.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/napi-leaks-suppression.txt b/test/napi-leaks-suppression.txt index c728b31..c84b1c4 100644 --- a/test/napi-leaks-suppression.txt +++ b/test/napi-leaks-suppression.txt @@ -2,6 +2,10 @@ # (ie they happen only a fixed number of times per process execution) # The list is maintained for Node.js 18 +# Ignore Python which does not have symbols +# Alas, this also covers up all stale Python references +leak:libpython3 + # Known leaks in Node-API leak:napi_module_register leak:napi_register_module_v1 From d53e3067f84d25f5ca8074eba3cf257044c493ab Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 3 Oct 2024 17:36:28 +0200 Subject: [PATCH 7/7] ignore the addon modules too --- test/napi-leaks-suppression.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/napi-leaks-suppression.txt b/test/napi-leaks-suppression.txt index c84b1c4..e461dd8 100644 --- a/test/napi-leaks-suppression.txt +++ b/test/napi-leaks-suppression.txt @@ -5,6 +5,7 @@ # Ignore Python which does not have symbols # Alas, this also covers up all stale Python references leak:libpython3 +leak:site-packages # Known leaks in Node-API leak:napi_module_register