diff --git a/CHANGELOG.md b/CHANGELOG.md index 8416d813..86a5ad63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [v0.9.6] - 2026-XX-XX + +### Fixed + +- Replace re2-wasm with re2js in library/regex-utilities.js to eliminate the underlying WASM-heap leak + +### Tx Conformance Statement + +XXX + ## [v0.9.5] - 2026-05-16 ### Fixed - Workaround for memory leak in re2-wasm library that reduces it's severity +- Replace re2-wasm with re2js in library/regex-utilities.js to eliminate the underlying WASM-heap leak ### Tx Conformance Statement diff --git a/config-template.json b/config-template.json index 7aba822a..802d2632 100644 --- a/config-template.json +++ b/config-template.json @@ -63,7 +63,8 @@ "enabled": true, "database": "/absolute/path/to/publisher.db", "sessionSecret": "change-this-to-a-secure-random-string", - "workspaceRoot": "/absolute/path/to/workspaces" + "workspaceRoot": "/absolute/path/to/workspaces", + "igPublisherTimeoutMinutes": 60 }, "npmprojector": { "enabled": true, diff --git a/library/regex-utilities.js b/library/regex-utilities.js index c289e4d5..f8772811 100644 --- a/library/regex-utilities.js +++ b/library/regex-utilities.js @@ -1,27 +1,64 @@ -const { RE2 } = require('re2-wasm'); +const { RE2JS } = require('re2js'); + +// Translate a JS-style flags string ("i", "im", "is", etc.) into the bit-flag +// integer that RE2JS.compile() expects. Only the flags actually used by callers +// (and their natural counterparts) are mapped; anything else is ignored. +function toRE2JSFlags(flags) { + if (!flags) return 0; + let bits = 0; + if (flags.includes('i')) bits |= RE2JS.CASE_INSENSITIVE; + if (flags.includes('m')) bits |= RE2JS.MULTILINE; + if (flags.includes('s')) bits |= RE2JS.DOTALL; + // The 'u' flag was a re2-wasm workaround; re2js is pure JS and handles + // unicode natively, so it's a no-op here. + return bits; +} + +// Thin wrapper that exposes the only method the rest of the codebase uses +// against compiled regexes: a JS-RegExp-style `.test(input)` that returns +// true if the pattern is found anywhere in `input`. +class CompiledRegex { + constructor(pattern, flags) { + this._pattern = RE2JS.compile(pattern, toRE2JSFlags(flags)); + } + + test(input) { + if (input == null) return false; + return this._pattern.matcher(String(input)).find(); + } +} class RegExUtilities { + // re2js doesn't have re2-wasm's WASM-heap leak, so the cache here is + // purely a performance optimisation. We cap it with a simple LRU so a + // workload with many distinct regex patterns can't grow it without bound. + static MAX_CACHE_SIZE = 1000; + constructor() { this._cache = new Map(); } compile(pattern, flags) { - // re2-wasm has a fixed 16 MB WASM heap with no real free(): every - // new RE2(...) permanently consumes a few KB. Cache by (pattern, flags) - // so the same regex compiles at most once. - // TODO: replace re2-wasm with native re2 to eliminate the underlying leak. - const re2Flags = flags && flags.includes('u') ? flags : (flags || '') + 'u'; - const key = pattern + '|' + re2Flags; - let compiled = this._cache.get(key); - if (!compiled) { - compiled = new RE2(pattern, re2Flags); - this._cache.set(key, compiled); + const key = pattern + '|' + (flags || ''); + const cached = this._cache.get(key); + if (cached) { + // Move to most-recently-used position by re-inserting. + this._cache.delete(key); + this._cache.set(key, cached); + return cached; + } + const compiled = new CompiledRegex(pattern, flags); + if (this._cache.size >= RegExUtilities.MAX_CACHE_SIZE) { + // Evict the oldest entry. Map iteration order is insertion order, so + // the first key is the least-recently-used. + const oldest = this._cache.keys().next().value; + this._cache.delete(oldest); } + this._cache.set(key, compiled); return compiled; } } module.exports = new RegExUtilities(); - \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 9e2e7092..639b550f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "fhirsmith", - "version": "0.9.4", + "version": "0.9.5", "license": "BSD-3", "dependencies": { "axios": "^1.13.4", @@ -40,7 +40,7 @@ "passport-github2": "^0.1.12", "passport-google-oauth20": "^2.0.0", "properties-file": "^3.6.4", - "re2-wasm": "^1.0.2", + "re2js": "^2.8.0", "rimraf": "^5.0.10", "sqlite3": "^5.1.7", "tar": "^7.5.7", @@ -8060,14 +8060,11 @@ "node": ">=0.10.0" } }, - "node_modules/re2-wasm": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/re2-wasm/-/re2-wasm-1.0.2.tgz", - "integrity": "sha512-VXUdgSiUrE/WZXn6gUIVVIsg0+Hp6VPZPOaHCay+OuFKy6u/8ktmeNEf+U5qSA8jzGGFsg8jrDNu1BeHpz2pJA==", - "license": "Apache-2.0", - "engines": { - "node": ">=10" - } + "node_modules/re2js": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/re2js/-/re2js-2.8.0.tgz", + "integrity": "sha512-HCjH9S3vdPyoYqv6++dZGa0RtG9xr7cAV00I0ymNHaKgiYOISsfTOMAj2UAP9miRWP+786AO/a4bIvtzTFOI1Q==", + "license": "MIT" }, "node_modules/react-is": { "version": "18.3.1", diff --git a/package.json b/package.json index 9afb5dda..0feecaea 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "passport-github2": "^0.1.12", "passport-google-oauth20": "^2.0.0", "properties-file": "^3.6.4", - "re2-wasm": "^1.0.2", + "re2js": "^2.8.0", "rimraf": "^5.0.10", "sqlite3": "^5.1.7", "tar": "^7.5.7", diff --git a/publisher/publisher.js b/publisher/publisher.js index 415807c4..a0089a86 100644 --- a/publisher/publisher.js +++ b/publisher/publisher.js @@ -649,13 +649,14 @@ class PublisherModule { reject(error); }); - // Timeout after 30 minutes + // Timeout configurable via publisher.igPublisherTimeoutMinutes (default: 60 minutes) + const timeoutMinutes = this.config.igPublisherTimeoutMinutes || 60; const timeout = setTimeout(async () => { java.kill(); logStream.end(); - await this.logTaskMessage(taskId, 'error', 'IG Publisher timed out after 30 minutes'); + await this.logTaskMessage(taskId, 'error', 'IG Publisher timed out after ' + timeoutMinutes + ' minutes'); reject(new Error('IG Publisher timed out')); - }, 30 * 60 * 1000); + }, timeoutMinutes * 60 * 1000); java.on('close', () => { clearTimeout(timeout); diff --git a/publisher/task-draft.js b/publisher/task-draft.js index 1a717ac0..2cabcc3f 100644 --- a/publisher/task-draft.js +++ b/publisher/task-draft.js @@ -191,6 +191,7 @@ class DraftTaskProcessor { stdio: ['pipe', 'pipe', 'pipe'] }); + // eslint-disable-next-line no-unused-vars let stdout = ''; let stderr = ''; @@ -255,6 +256,7 @@ class DraftTaskProcessor { stdio: ['pipe', 'pipe', 'pipe'] }); + // eslint-disable-next-line no-unused-vars let stdout = ''; let stderr = ''; @@ -316,6 +318,7 @@ class DraftTaskProcessor { }); let stdout = ''; + // eslint-disable-next-line no-unused-vars let stderr = ''; sushi.stdout.on('data', (data) => { @@ -382,6 +385,7 @@ class DraftTaskProcessor { logStream.write('Working Directory: ' + draftDir + '\n'); logStream.write('=====================================\n\n'); + // eslint-disable-next-line no-unused-vars let hasOutput = false; let lastProgressUpdate = Date.now(); @@ -433,7 +437,8 @@ class DraftTaskProcessor { reject(error); }); - // Timeout after 30 minutes + // Timeout configurable via publisher.igPublisherTimeoutMinutes (default: 60 minutes) + const timeoutMinutes = this.config.igPublisherTimeoutMinutes || 60; const timeout = setTimeout(async () => { java.kill('SIGTERM'); // Try graceful shutdown first @@ -442,11 +447,11 @@ class DraftTaskProcessor { java.kill('SIGKILL'); }, 10000); - logStream.write('\nTIMEOUT: Process killed after 30 minutes\n'); + logStream.write('\nTIMEOUT: Process killed after ' + timeoutMinutes + ' minutes\n'); logStream.end(); - await this.logTaskMessage(taskId, 'error', 'IG Publisher timed out after 30 minutes'); + await this.logTaskMessage(taskId, 'error', 'IG Publisher timed out after ' + timeoutMinutes + ' minutes'); reject(new Error('IG Publisher timed out')); - }, 30 * 60 * 1000); + }, timeoutMinutes * 60 * 1000); java.on('close', () => { clearTimeout(timeout); diff --git a/test-scripts/repro-re2-wasm-leak.js b/test-scripts/repro-re2-wasm-leak.js index 4742e2fe..5ac77e0e 100644 --- a/test-scripts/repro-re2-wasm-leak.js +++ b/test-scripts/repro-re2-wasm-leak.js @@ -1,14 +1,15 @@ -// Manual reproducer for the re2-wasm WASM heap leak. -// See library/regex-utilities.js for the workaround. +// Historical reproducer for the re2-wasm WASM heap leak. +// regex-utilities.js now sits on top of re2js (a pure-JS RE2 port), so the +// underlying leak is gone. Kept as a stress test for the compile cache: // // node test-scripts/repro-re2-wasm-leak.js # same-pattern stress // node test-scripts/repro-re2-wasm-leak.js --unique # unique-pattern stress // -// Without the cache, same-pattern OOMs at ~2965 iterations. -// With the cache, same-pattern runs indefinitely. -// Unique-pattern still OOMs (each pattern is a real compile and the underlying -// re2-wasm heap leak still applies). A proper fix is to replace re2-wasm with -// the native `re2` package. +// Background (re2-wasm era): without the cache, same-pattern OOMed at ~2965 +// iterations; with the cache, same-pattern ran indefinitely but unique-pattern +// still OOMed because every distinct pattern was a real compile and re2-wasm +// could not free its WASM heap. Under re2js both modes now run indefinitely +// (unique-pattern is bounded only by ordinary V8 heap growth from the cache). const re = require('../library/regex-utilities'); diff --git a/tests/cs/cs-cs.test.js b/tests/cs/cs-cs.test.js index a7140321..e6e98f28 100644 --- a/tests/cs/cs-cs.test.js +++ b/tests/cs/cs-cs.test.js @@ -1520,7 +1520,7 @@ describe('FHIR CodeSystem Provider', () => { test('should handle invalid regex gracefully', async () => { await expect( simpleProvider.filter(filterContext, true, 'code', 'regex', '[invalid') - ).rejects.toThrow('The regex \'[invalid\' is not valid: Invalid regular expression: /^[invalid$/u: missing ]: [invalid$'); + ).rejects.toThrow('The regex \'[invalid\' is not valid: error parsing regexp: missing closing ]: `[invalid$`'); }); }); diff --git a/tests/cs/cs-omop.test.js b/tests/cs/cs-omop.test.js index 9bf71efc..b05cb996 100644 --- a/tests/cs/cs-omop.test.js +++ b/tests/cs/cs-omop.test.js @@ -94,8 +94,8 @@ describe('OMOP Provider', () => { expect(provider).toBeInstanceOf(OMOPServices); }); - test('should check database status', () => { - const status = OMOPServicesFactory.checkDB(testDbPath); + test('should check database status', async () => { + const status = await OMOPServicesFactory.checkDB(testDbPath); expect(status).toContain('OK'); }); diff --git a/tests/data/UcumFunctionalTests.xml b/tests/data/UcumFunctionalTests.xml index 76a7fb03..dc4a0e10 100644 --- a/tests/data/UcumFunctionalTests.xml +++ b/tests/data/UcumFunctionalTests.xml @@ -383,6 +383,7 @@ + diff --git a/tx/cs/cs-loinc.js b/tx/cs/cs-loinc.js index bf187c25..d6615d2c 100644 --- a/tx/cs/cs-loinc.js +++ b/tx/cs/cs-loinc.js @@ -1429,8 +1429,8 @@ class LoincServicesFactory extends CodeSystemFactoryProvider { * @returns {Promise} Array of {code, display} objects */ async #getAnswerListConcepts(sourceKey) { - return new Promise((resolve, reject) => { - let db = new sqlite3.Database(this.dbPath); + const db = new sqlite3.Database(this.dbPath, sqlite3.OPEN_READONLY); + try { const sql = ` SELECT Code, Description FROM Relationships, Codes @@ -1439,17 +1439,17 @@ class LoincServicesFactory extends CodeSystemFactoryProvider { AND Relationships.TargetKey = Codes.CodeKey `; - db.all(sql, [sourceKey], (err, rows) => { - if (err) { - reject(err); - } else { - const concepts = rows.map(row => ({ - code: row.Code - })); - resolve(concepts); - } + const rows = await new Promise((resolve, reject) => { + db.all(sql, [sourceKey], (err, result) => { + if (err) reject(err); + else resolve(result); + }); }); - }); + + return rows.map(row => ({ code: row.Code })); + } finally { + await new Promise((resolve) => db.close(() => resolve())); + } } id() { diff --git a/tx/cs/cs-omop.js b/tx/cs/cs-omop.js index 866281c7..125cf153 100644 --- a/tx/cs/cs-omop.js +++ b/tx/cs/cs-omop.js @@ -906,41 +906,42 @@ class OMOPServicesFactory extends CodeSystemFactoryProvider { return new OMOPServices(opContext, supplements, db, this._sharedData); } - static checkDB(dbPath) { + static async checkDB(dbPath) { + const fs = require('fs'); try { - const fs = require('fs'); - - // Check if file exists if (!fs.existsSync(dbPath)) { return 'Database file not found'; } - - // Check file size const stats = fs.statSync(dbPath); if (stats.size < 1024) { return 'Database file too small'; } + } catch (e) { + return `Database error: ${e.message}`; + } - // Try to open database and check for required tables - const db = new sqlite3.Database(dbPath); - - try { - // Simple count query to verify database integrity - db.get('SELECT COUNT(*) as count FROM Concepts', (err) => { - if (err) { - db.close(); - return 'Missing Tables - needs re-importing (by java)'; - } - }); - - db.close(); - return 'OK (check via provider for count)'; - } catch (e) { - return 'Missing Tables - needs re-importing (by java)'; - } + let db; + try { + db = new sqlite3.Database(dbPath, sqlite3.OPEN_READONLY); } catch (e) { return `Database error: ${e.message}`; } + + try { + // Simple count query to verify database integrity. If the Concepts + // table is missing, db.get rejects and we fall through to the catch. + const row = await new Promise((resolve, reject) => { + db.get('SELECT COUNT(*) as count FROM Concepts', (err, result) => { + if (err) reject(err); + else resolve(result); + }); + }); + return `OK (${row && row.count != null ? row.count : 0} Concepts)`; + } catch (_e) { + return 'Missing Tables - needs re-importing (by java)'; + } finally { + await new Promise((resolve) => db.close(() => resolve())); + } } diff --git a/tx/cs/cs-unii.js b/tx/cs/cs-unii.js index e714be9c..26129810 100644 --- a/tx/cs/cs-unii.js +++ b/tx/cs/cs-unii.js @@ -210,18 +210,18 @@ class UniiServicesFactory extends CodeSystemFactoryProvider { } async load() { - let db = new sqlite3.Database(this.dbPath); - - return new Promise((resolve, reject) => { - db.get('SELECT Version FROM UniiVersion', (err, row) => { - if (err) { - reject(new Error(err)); - } else { - this._version = row ? row.Version : 'unknown'; - resolve(); // This resolves the Promise - } + const db = new sqlite3.Database(this.dbPath, sqlite3.OPEN_READONLY); + try { + const row = await new Promise((resolve, reject) => { + db.get('SELECT Version FROM UniiVersion', (err, result) => { + if (err) reject(new Error(err)); + else resolve(result); + }); }); - }); + this._version = row ? row.Version : 'unknown'; + } finally { + await new Promise((resolve) => db.close(() => resolve())); + } } defaultVersion() { diff --git a/tx/operation-context.js b/tx/operation-context.js index 5ce14df0..b37f4f14 100644 --- a/tx/operation-context.js +++ b/tx/operation-context.js @@ -448,6 +448,11 @@ class OperationContext { this.resourceCache = resourceCache; this.expansionCache = expansionCache; this.debugging = isDebugging(); + // Providers opened during this operation that need their underlying + // resources (sqlite connections, etc.) released when the operation ends. + // Shared by reference with copy()'d contexts so a sub-operation's + // providers are cleaned up by the parent request's closeProviders(). + this._openProviders = []; this.timeTracker.step('tx-op'); } @@ -476,6 +481,9 @@ class OperationContext { newContext.logEntries = [...this.logEntries]; newContext.debugging = this.debugging; newContext.usageTracker = this.usageTracker; + // Share the same provider-cleanup list so providers opened by the copy + // are released when the parent operation ends. + newContext._openProviders = this._openProviders; return newContext; } @@ -624,6 +632,37 @@ class OperationContext { return this.id; } + /** + * Register a code-system provider whose resources (typically a sqlite + * connection opened by factory.build()) should be released when the + * operation ends. Providers without a close() method are ignored. + * @param {Object} provider - The provider returned from factory.build() + */ + registerProvider(provider) { + if (provider && typeof provider.close === 'function') { + this._openProviders.push(provider); + } + } + + /** + * Close every provider registered during this operation. Safe to call + * multiple times — the list is cleared after the first call. Errors + * from individual close() calls are swallowed so one bad provider can't + * prevent the others from releasing their resources. + */ + async closeProviders() { + if (!this._openProviders || this._openProviders.length === 0) return; + const providers = this._openProviders; + this._openProviders = []; + for (const p of providers) { + try { + await p.close(); + } catch (_e) { + // Swallow — provider cleanup is best-effort. + } + } + } + /** * @type {Languages} languages specified in request */ diff --git a/tx/problems.js b/tx/problems.js index 07ce10e4..455cce8e 100644 --- a/tx/problems.js +++ b/tx/problems.js @@ -2,10 +2,6 @@ const escape = require('escape-html'); class ProblemFinder { - constructor() { - this.map = new Map(); - } - async scanValueSets(provider) { let unknownVersions = {}; // system -> Set of versions not known to the server for (let vsp of provider.valueSetProviders) { diff --git a/tx/provider.js b/tx/provider.js index 3aa9fc9f..162cabc5 100644 --- a/tx/provider.js +++ b/tx/provider.js @@ -419,6 +419,7 @@ class Provider { } if (factory != null) { const csp = await factory.build(opContext, []); + opContext.registerProvider(csp); const c = csp ? csp.locate(code) : null; if (c) { if (factory.iteratable()) { diff --git a/tx/tx.js b/tx/tx.js index 2f1964e9..28994f83 100644 --- a/tx/tx.js +++ b/tx/tx.js @@ -306,6 +306,18 @@ class TXModule { req.txI18n = this.i18n; req.txLog = this.log; + // Release any code-system providers that opened sqlite connections + // during this request. closeProviders() is idempotent so it's safe + // for both events to fire. Listeners are sync; the close itself + // runs fire-and-forget on the event loop. + const releaseProviders = () => { + opContext.closeProviders().catch((err) => { + try { this.log.warn(`closeProviders failed: ${err && err.message}`); } catch (_) { /* ignore */ } + }); + }; + res.on('finish', releaseProviders); + res.on('close', releaseProviders); + // Add X-Request-Id header to response res.setHeader('X-Request-Id', requestId); diff --git a/tx/workers/translate.js b/tx/workers/translate.js index f038f5cb..d7547a3c 100644 --- a/tx/workers/translate.js +++ b/tx/workers/translate.js @@ -495,6 +495,7 @@ class TranslateWorker extends TerminologyWorker { let result = false; const factory = cm.jsonObj.internalSource; let prov = await factory.build(this.opContext, []); + this.opContext.registerProvider(prov); output.push({ name: 'used-system', diff --git a/tx/workers/worker.js b/tx/workers/worker.js index a9676c34..5df09aaa 100644 --- a/tx/workers/worker.js +++ b/tx/workers/worker.js @@ -184,6 +184,10 @@ class TerminologyWorker { if (checkVer) { this.checkVersion(url, provider.version(), params, provider.versionAlgorithm(), op); } + // Track for lifecycle cleanup at end of request. Providers built from + // a factory open a fresh sqlite connection that needs releasing; + // resource-built providers and others without close() are no-ops. + this.opContext.registerProvider(provider); } return provider;