From 823cdf70caad5ce1a9813bf31b50062d025dfcf2 Mon Sep 17 00:00:00 2001 From: Snarling <84951833+Snarling@users.noreply.github.com> Date: Fri, 19 Aug 2022 18:21:31 -0400 Subject: [PATCH] Fix compile race conditions --- src/NetscriptFunctions.ts | 25 ++-------- src/NetscriptJSEvaluator.ts | 50 ++++++++++--------- src/Script/Script.ts | 7 +-- ...termineAllPossibilitiesForTabCompletion.ts | 6 +-- 4 files changed, 37 insertions(+), 51 deletions(-) diff --git a/src/NetscriptFunctions.ts b/src/NetscriptFunctions.ts index d6998449e..dfb9dc060 100644 --- a/src/NetscriptFunctions.ts +++ b/src/NetscriptFunctions.ts @@ -1465,34 +1465,19 @@ const base: InternalAPI = { }, write: (ctx: NetscriptContext) => - (_port: unknown, data: unknown = "", _mode: unknown = "a"): void => { + (_port: unknown, _data: unknown = "", _mode: unknown = "a"): void => { const port = helpers.string(ctx, "port", _port); + const data = helpers.string(ctx, "data", _data); const mode = helpers.string(ctx, "mode", _mode); if (isString(port)) { // Write to script or text file let fn = port; - if (!isValidFilePath(fn)) { - throw helpers.makeRuntimeErrorMsg(ctx, `Invalid filepath: ${fn}`); - } + if (!isValidFilePath(fn)) throw helpers.makeRuntimeErrorMsg(ctx, `Invalid filepath: ${fn}`); - if (fn.lastIndexOf("/") === 0) { - fn = removeLeadingSlash(fn); - } + if (fn.lastIndexOf("/") === 0) fn = removeLeadingSlash(fn); - // Coerce 'data' to be a string - try { - data = String(data); - } catch (e: unknown) { - throw helpers.makeRuntimeErrorMsg( - ctx, - `Invalid data (${String(e)}). Data being written must be convertible to a string`, - ); - } + const server = helpers.getServer(ctx, ctx.workerScript.hostname); - const server = ctx.workerScript.getServer(); - if (server == null) { - throw helpers.makeRuntimeErrorMsg(ctx, "Error getting Server. This is a bug. Report to dev."); - } if (isScriptFilename(fn)) { // Write to script let script = ctx.workerScript.getScriptOnServer(fn, server); diff --git a/src/NetscriptJSEvaluator.ts b/src/NetscriptJSEvaluator.ts index 46a9ef5c1..e7fff05b0 100644 --- a/src/NetscriptJSEvaluator.ts +++ b/src/NetscriptJSEvaluator.ts @@ -11,6 +11,8 @@ import { WorkerScript } from "./Netscript/WorkerScript"; import { Script } from "./Script/Script"; import { areImportsEquals } from "./Terminal/DirectoryHelpers"; import { IPlayer } from "./PersonObjects/IPlayer"; +import { ScriptModule } from "./Script/ScriptModule"; +import { queue } from "jquery"; // Acorn type def is straight up incomplete so we have to fill with our own. export type Node = any; @@ -20,8 +22,23 @@ function makeScriptBlob(code: string): Blob { return new Blob([code], { type: "text/javascript" }); } -export async function compile(player: IPlayer, script: Script, scripts: Script[]): Promise { - if (!shouldCompile(script, scripts)) return; +export async function compile(player: IPlayer, script: Script, scripts: Script[]): Promise { + //!shouldCompile ensures that script.module is non-null, hence the "as". + if (!shouldCompile(script, scripts)) return script.module as Promise; + script.queueCompile = true; + //If we're already in the middle of compiling (script.module has not resolved yet), wait for the previous compilation to finish + //If script.module is null, this does nothing. + await script.module; + //If multiple compiles were called on the same script before a compilation could be completed this ensures only one complilation is actually performed. + if (!script.queueCompile) return script.module as Promise; + script.queueCompile = false; + script.updateRamUsage(player, scripts); + const uurls = _getScriptUrls(script, scripts, []); + const url = uurls[uurls.length - 1].url; + if (script.url && script.url !== url) URL.revokeObjectURL(script.url); + + if (script.dependencies.length > 0) script.dependencies.forEach((dep) => URL.revokeObjectURL(dep.url)); + script.url = uurls[uurls.length - 1].url; // The URL at the top is the one we want to import. It will // recursively import all the other modules in the urlStack. // @@ -29,27 +46,9 @@ export async function compile(player: IPlayer, script: Script, scripts: Script[] // but not really behaves like import. Particularly, it cannot // load fully dynamic content. So we hide the import from webpack // by placing it inside an eval call. - script.updateRamUsage(player, scripts); - const uurls = _getScriptUrls(script, scripts, []); - const url = uurls[uurls.length - 1].url; - if (script.url && script.url !== url) { - URL.revokeObjectURL(script.url); - // Thoughts: Should we be revoking any URLs here? - // If a script is modified repeatedly between two states, - // we could reuse the blob at a later time. - // BlobCache.removeByValue(script.url); - // URL.revokeObjectURL(script.url); - // if (script.dependencies.length > 0) { - // script.dependencies.forEach((dep) => { - // removeBlobFromCache(dep.url); - // URL.revokeObjectURL(dep.url); - // }); - // } - } - if (script.dependencies.length > 0) script.dependencies.forEach((dep) => URL.revokeObjectURL(dep.url)); - script.url = uurls[uurls.length - 1].url; script.module = new Promise((resolve) => resolve(eval("import(uurls[uurls.length - 1].url)"))); script.dependencies = uurls; + return script.module; } // Begin executing a user JS script, and return a promise that resolves @@ -67,9 +66,8 @@ export async function executeJSScript( ): Promise { const script = workerScript.getScript(); if (script === null) throw new Error("script is null"); - await compile(player, script, scripts); + const loadedModule = await compile(player, script, scripts); workerScript.ramUsage = script.ramUsage; - const loadedModule = await script.module; const ns = workerScript.env.vars; @@ -113,7 +111,11 @@ function isDependencyOutOfDate(filename: string, scripts: Script[], scriptModule */ function shouldCompile(script: Script, scripts: Script[]): boolean { if (!script.module) return true; - return script.dependencies.some((dep) => isDependencyOutOfDate(dep.filename, scripts, script.moduleSequenceNumber)); + if (script.dependencies.some((dep) => isDependencyOutOfDate(dep.filename, scripts, script.moduleSequenceNumber))) { + script.module = null; + return true; + } + return false; } // Gets a stack of blob urls, the top/right-most element being diff --git a/src/Script/Script.ts b/src/Script/Script.ts index ba0079c8c..e70c0433b 100644 --- a/src/Script/Script.ts +++ b/src/Script/Script.ts @@ -46,15 +46,16 @@ export class Script { ramUsage = 0; ramUsageEntries?: RamUsageEntry[]; + // Used to deconflict multiple simultaneous compilations. + queueCompile = false; + // hostname of server that this script is on. server = ""; constructor(player: IPlayer | null = null, fn = "", code = "", server = "", otherScripts: Script[] = []) { this.filename = fn; this.code = code; - this.ramUsage = 0; this.server = server; // hostname of server this script is on - this.module = null; this.moduleSequenceNumber = ++globalModuleSequenceNumber; if (this.code !== "" && player !== null) { this.updateRamUsage(player, otherScripts); @@ -105,7 +106,7 @@ export class Script { const [dependentScript] = otherScripts.filter( (s) => s.filename === dependent.filename && s.server == dependent.server, ); - if (dependentScript !== null) dependentScript.markUpdated(); + dependentScript?.markUpdated(); } } diff --git a/src/Terminal/determineAllPossibilitiesForTabCompletion.ts b/src/Terminal/determineAllPossibilitiesForTabCompletion.ts index 81a94390f..7355c7d1d 100644 --- a/src/Terminal/determineAllPossibilitiesForTabCompletion.ts +++ b/src/Terminal/determineAllPossibilitiesForTabCompletion.ts @@ -285,10 +285,8 @@ export async function determineAllPossibilitiesForTabCompletion( return processFilepath(script.filename) === fn || script.filename === "/" + fn; }); if (!script) return; // Doesn't exist. - if (!script.module) { - await compile(p, script, currServ.scripts); - } - const loadedModule = await script.module; + //Will return the already compiled module if recompilation not needed. + const loadedModule = await compile(p, script, currServ.scripts); if (!loadedModule || !loadedModule.autocomplete) return; // Doesn't have an autocomplete function. const runArgs = { "--tail": Boolean, "-t": Number };