From ecf18f09d08416300a8373306a2a422ea1b70dd7 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 4 Mar 2025 15:04:13 -0500 Subject: [PATCH 1/2] Fix crash in bsc project when thread has error --- src/bsc/BscProjectThreaded.ts | 31 ++++++++++++++++++-- src/debugSession/BrightScriptDebugSession.ts | 21 ++++++++----- src/util.ts | 23 ++++++++++++--- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/src/bsc/BscProjectThreaded.ts b/src/bsc/BscProjectThreaded.ts index fbde2cd2..91ce3a1a 100644 --- a/src/bsc/BscProjectThreaded.ts +++ b/src/bsc/BscProjectThreaded.ts @@ -18,6 +18,7 @@ export class BscProjectThreaded implements ExtractMethods { private messageHandler: ThreadMessageHandler; private activateDeferred = new Deferred(); + private errorDeferred = new Deferred(); /** * Has this project finished activating (either resolved or rejected...) @@ -26,12 +27,38 @@ export class BscProjectThreaded implements ExtractMethods { return this.activateDeferred.isCompleted; } + /** + * If an error occurs at any time during the worker's lifetime, it will be caught and stored here. + */ + public isErrored() { + return this.errorDeferred.isCompleted; + } + + private ready() { + return Promise.race([ + //if we've encountered an error, reject immediately. The underlying error should propagate up + this.errorDeferred.promise, + //wait for the activate to finish. This should typically be the only promise that resolves + this.activateDeferred.promise + ]); + } + public async activate(options: Parameters[0]) { const timeEnd = this.logger.timeStart('log', 'activate'); // start a new worker thread or get an unused existing thread this.worker = bscProjectWorkerPool.getWorker(); + //!!!IMPORTANT!!! this observer must be registered in order to prevent the worker thread from crashing the main thread + this.worker.on('error', (error) => { + this.logger.error('Worker encountered an error', error); + this.errorDeferred.reject(error); + //!!!IMPORTANT!!! this is required to prevent node from freaking out about an uncaught promise + this.errorDeferred.promise.catch(e => { + //do nothing. this is just to prevent node from freaking out about an uncaught promise + }); + }); + //link the message handler to the worker this.messageHandler = new ThreadMessageHandler({ name: 'MainThread', @@ -62,7 +89,7 @@ export class BscProjectThreaded implements ExtractMethods { * @param relativePath path to the file relative to rootDir * @returns */ - public getScopeFunctionsForFile(options: { relativePath: string }): MaybePromise> { + public async getScopeFunctionsForFile(options: { relativePath: string }): Promise> { return this.sendStandardRequest('getScopeFunctionsForFile', options); } @@ -73,7 +100,7 @@ export class BscProjectThreaded implements ExtractMethods { * @returns the response from the request */ private async sendStandardRequest(name: MethodNames, ...data: any[]) { - await this.activateDeferred.promise; + await this.ready(); const response = await this.messageHandler.sendRequest(name, { data: data }); diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 49215c16..283e3b3b 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -1864,15 +1864,20 @@ export class BrightScriptDebugSession extends BaseDebugSession { } const frame = this.rokuAdapter.getStackFrameById(args.frameId); - let scopeFunctions = await this.projectManager.getScopeFunctionsForFile(frame.filePath as string); - for (let scopeFunction of scopeFunctions) { - if (!completions.has(`${scopeFunction.completionItemKind}-${scopeFunction.name.toLocaleLowerCase()}`)) { - completions.set(`${scopeFunction.completionItemKind}-${scopeFunction.name.toLocaleLowerCase()}`, { - label: scopeFunction.name, - type: scopeFunction.completionItemKind, - sortText: '000000' - }); + + try { + let scopeFunctions = await this.projectManager.getScopeFunctionsForFile(frame.filePath as string); + for (let scopeFunction of scopeFunctions) { + if (!completions.has(`${scopeFunction.completionItemKind}-${scopeFunction.name.toLocaleLowerCase()}`)) { + completions.set(`${scopeFunction.completionItemKind}-${scopeFunction.name.toLocaleLowerCase()}`, { + label: scopeFunction.name, + type: scopeFunction.completionItemKind, + sortText: '000000' + }); + } } + } catch (e) { + this.logger.warn('Could not build list of scope functions for file', e); } } } diff --git a/src/util.ts b/src/util.ts index 51e0ff6d..49e43d74 100644 --- a/src/util.ts +++ b/src/util.ts @@ -14,6 +14,7 @@ import type { Response } from 'request'; import type * as requestType from 'request'; import { OutputEvent } from '@vscode/debugadapter'; import * as xml2js from 'xml2js'; +import { isPromise } from 'util/types'; const request = r as typeof requestType; class Util { @@ -499,12 +500,26 @@ class Util { * @param disposables a list of functions or disposables */ public applyDispose(disposables: DisposableLike[]) { + let promises = []; + let exceptions = []; + disposables ??= []; for (const disposable of disposables) { - if (typeof disposable === 'function') { - disposable(); - } else { - disposable?.dispose?.(); + let value: any; + try { + if (typeof disposable === 'function') { + value = disposable(); + } else { + value = disposable?.dispose?.(); + } + } catch (e) { + exceptions.push(e); + } + //if this value is a promise, add a .catch to it so we don't bring down the app + if (isPromise(value)) { + value.catch(e => { + console.error('Unhandled promise during dispose', e); + }); } } //empty the array From 5a002a4863b71f506242bfe1fc2bdf76638d79e9 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 4 Mar 2025 15:07:06 -0500 Subject: [PATCH 2/2] Merge branch 'master' of https://github.com/rokucommunity/roku-debug into fix-bsc-project-crash