diff --git a/config/eslint-config-ironfish/index.js b/config/eslint-config-ironfish/index.js index 978bcb75d7..d0747feae8 100644 --- a/config/eslint-config-ironfish/index.js +++ b/config/eslint-config-ironfish/index.js @@ -68,6 +68,7 @@ module.exports = { rules: { 'ironfish/no-vague-imports': 'error', 'ironfish/no-buffer-cmp': 'error', + 'ironfish/no-promise-race': 'error', // Catches expressions that aren't assigned '@typescript-eslint/no-unused-expressions': [ diff --git a/config/eslint-plugin-ironfish/index.js b/config/eslint-plugin-ironfish/index.js index 0ff7d24290..8b189ee5b5 100644 --- a/config/eslint-plugin-ironfish/index.js +++ b/config/eslint-plugin-ironfish/index.js @@ -42,4 +42,19 @@ module.exports.rules = { }; }, }, + "no-promise-race": { + create(context) { + return { + MemberExpression: function (node) { + if (node.object.name === 'Promise' && node.property.name === 'race') { + context.report({ + node, + message: + "Promise.race leaks memory. You can work around it by using PromiseUtils.split to pass resolve/reject to other Promises. See https://github.com/nodejs/node/issues/17469#issuecomment-685216777 for more details.", + }); + } + }, + }; + }, + }, }; diff --git a/ironfish/src/utils/asyncQueue.test.ts b/ironfish/src/utils/asyncQueue.test.ts index 52e9239597..582a7a03d1 100644 --- a/ironfish/src/utils/asyncQueue.test.ts +++ b/ironfish/src/utils/asyncQueue.test.ts @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ import { AsyncQueue } from './asyncQueue' +import { PromiseUtils } from './promise' describe('AsyncQueue', () => { it('yields items in the same order as they were added', async () => { @@ -81,7 +82,11 @@ describe('AsyncQueue', () => { () => 'otherPromise', ) - const resolved = await Promise.race([pushPromise, otherPromise]) + const [promise, res, rej] = PromiseUtils.split() + pushPromise.then(res, rej) + otherPromise.then(res, rej) + + const resolved = await promise expect(resolved).toBe('otherPromise') // After popping an element, the promise returned by push should resolve @@ -101,7 +106,11 @@ describe('AsyncQueue', () => { () => 'otherPromise', ) - const resolved = await Promise.race([popPromise, otherPromise]) + const [promise, res, rej] = PromiseUtils.split() + popPromise.then(res, rej) + otherPromise.then(res, rej) + + const resolved = await promise expect(resolved).toBe('otherPromise') // After pushing a new element, the promise returned by pop should diff --git a/ironfish/src/wallet/scanner/noteDecryptor.ts b/ironfish/src/wallet/scanner/noteDecryptor.ts index 3c80aed5dc..d74afc3de8 100644 --- a/ironfish/src/wallet/scanner/noteDecryptor.ts +++ b/ironfish/src/wallet/scanner/noteDecryptor.ts @@ -6,6 +6,7 @@ import { Config } from '../../fileStores' import { BlockHeader } from '../../primitives' import { Transaction } from '../../primitives/transaction' import { AsyncQueue } from '../../utils/asyncQueue' +import { PromiseUtils } from '../../utils/promise' import { WorkerPool } from '../../workerPool' import { Job } from '../../workerPool/job' import { @@ -23,6 +24,14 @@ export type DecryptNotesFromTransactionsCallback = ( transactions: Array<{ transaction: Transaction; decryptedNotes: Array }>, ) => Promise +type DecryptQueueValue = { + job: Job + accounts: ReadonlyArray + blockHeader: BlockHeader + transactions: ReadonlyArray + callback: DecryptNotesFromTransactionsCallback +} + export class BackgroundNoteDecryptor { private isStarted = false @@ -34,13 +43,7 @@ export class BackgroundNoteDecryptor { private readonly workerPool: WorkerPool private readonly options: DecryptNotesOptions - private readonly decryptQueue: AsyncQueue<{ - job: Job - accounts: ReadonlyArray - blockHeader: BlockHeader - transactions: ReadonlyArray - callback: DecryptNotesFromTransactionsCallback - }> + private readonly decryptQueue: AsyncQueue constructor(workerPool: WorkerPool, config: Config, options: DecryptNotesOptions) { this.workerPool = workerPool @@ -81,13 +84,30 @@ export class BackgroundNoteDecryptor { } private async decryptLoop(): Promise { + let resolve: (value: DecryptQueueValue | void) => unknown + let reject: (reason?: unknown) => void + + this.onStopped.then( + (value) => resolve?.(value), + (err) => reject?.(err), + ) + while (this.isStarted) { if (this.decryptQueue.isEmpty() && this.triggerFlushed) { this.triggerFlushed() this.triggerFlushed = null } - const item = await Promise.race([this.decryptQueue.pop(), this.onStopped]) + const [promise, resolveNew, rejectNew] = PromiseUtils.split() + resolve = resolveNew + reject = rejectNew + + this.decryptQueue.pop().then( + (value) => resolve(value), + (err) => reject(err), + ) + + const item = await promise if (!item) { break }