fix: prevent memory leaks in experimental scripting sandbox (#5800)
- Cache and reuse a single `FaradayCage` WASM instance to avoid repeated allocations. - Dispose `InspectionService` watchers via `effectScope` to prevent accumulation on tab switch. - Use `Set` for environment variable key lookups in validation. - Dispose Monaco editor models on component unmount.
This commit is contained in:
parent
913863bd09
commit
645ecb55d8
8 changed files with 105 additions and 52 deletions
|
|
@ -141,6 +141,8 @@ onMounted(() => {
|
|||
})
|
||||
|
||||
onUnmounted(() => {
|
||||
editorModel.value?.dispose()
|
||||
|
||||
// Clean up context-specific type definitions for this editor instance
|
||||
contextTypeDefRef.value?.dispose()
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,17 @@ import {
|
|||
} from "@hoppscotch/data"
|
||||
import { refDebounced } from "@vueuse/core"
|
||||
import { Service } from "dioc"
|
||||
import { Component, Ref, ref, watch, computed, markRaw, reactive } from "vue"
|
||||
import {
|
||||
Component,
|
||||
Ref,
|
||||
ref,
|
||||
watch,
|
||||
computed,
|
||||
markRaw,
|
||||
reactive,
|
||||
effectScope,
|
||||
EffectScope,
|
||||
} from "vue"
|
||||
import { HoppRESTResponse } from "~/helpers/types/HoppRESTResponse"
|
||||
import { RESTTabService } from "../tab/rest"
|
||||
/**
|
||||
|
|
@ -112,8 +122,21 @@ export class InspectionService extends Service {
|
|||
|
||||
private readonly restTab = this.bind(RESTTabService)
|
||||
|
||||
private watcherStopHandle: (() => void) | null = null
|
||||
private effectScope: EffectScope | null = null
|
||||
|
||||
override onServiceInit() {
|
||||
this.initializeListeners()
|
||||
|
||||
// Watch for tab changes and inspector registration to reinitialize
|
||||
// and create new debounced refs
|
||||
watch(
|
||||
() => [this.inspectors.entries(), this.restTab.currentActiveTab.value.id],
|
||||
() => {
|
||||
this.initializeListeners()
|
||||
},
|
||||
{ flush: "pre" }
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -126,56 +149,59 @@ export class InspectionService extends Service {
|
|||
}
|
||||
|
||||
private initializeListeners() {
|
||||
watch(
|
||||
() => [this.inspectors.entries(), this.restTab.currentActiveTab.value.id],
|
||||
() => {
|
||||
const currentTabRequest = computed(() => {
|
||||
if (
|
||||
this.restTab.currentActiveTab.value.document.type === "test-runner"
|
||||
)
|
||||
return null
|
||||
// Dispose previous reactive effects
|
||||
this.watcherStopHandle?.()
|
||||
this.effectScope?.stop()
|
||||
|
||||
return this.restTab.currentActiveTab.value.document.type === "request"
|
||||
? this.restTab.currentActiveTab.value.document.request
|
||||
: this.restTab.currentActiveTab.value.document.response
|
||||
.originalRequest
|
||||
})
|
||||
// Create new effect scope for all computed refs and watchers
|
||||
this.effectScope = effectScope()
|
||||
|
||||
const currentTabResponse = computed(() => {
|
||||
if (this.restTab.currentActiveTab.value.document.type === "request") {
|
||||
return this.restTab.currentActiveTab.value.document.response
|
||||
}
|
||||
this.effectScope.run(() => {
|
||||
const currentTabRequest = computed(() => {
|
||||
if (this.restTab.currentActiveTab.value.document.type === "test-runner")
|
||||
return null
|
||||
})
|
||||
|
||||
const reqRef = computed(() => currentTabRequest.value)
|
||||
const resRef = computed(() => currentTabResponse.value)
|
||||
return this.restTab.currentActiveTab.value.document.type === "request"
|
||||
? this.restTab.currentActiveTab.value.document.request
|
||||
: this.restTab.currentActiveTab.value.document.response
|
||||
.originalRequest
|
||||
})
|
||||
|
||||
const debouncedReq = refDebounced(reqRef, 1000, { maxWait: 2000 })
|
||||
const debouncedRes = refDebounced(resRef, 1000, { maxWait: 2000 })
|
||||
const currentTabResponse = computed(() => {
|
||||
if (this.restTab.currentActiveTab.value.document.type === "request") {
|
||||
return this.restTab.currentActiveTab.value.document.response
|
||||
}
|
||||
return null
|
||||
})
|
||||
|
||||
const inspectorRefs = Array.from(this.inspectors.values()).map((x) =>
|
||||
// @ts-expect-error - This is a valid call
|
||||
const debouncedReq = refDebounced(currentTabRequest, 1000, {
|
||||
maxWait: 2000,
|
||||
})
|
||||
const debouncedRes = refDebounced(currentTabResponse, 1000, {
|
||||
maxWait: 2000,
|
||||
})
|
||||
|
||||
const inspectorRefs = computed(() =>
|
||||
Array.from(this.inspectors.values()).map((x) =>
|
||||
x.getInspections(debouncedReq, debouncedRes)
|
||||
)
|
||||
)
|
||||
|
||||
const activeInspections = computed(() =>
|
||||
inspectorRefs.flatMap((x) => x!.value)
|
||||
)
|
||||
const activeInspections = computed(() =>
|
||||
inspectorRefs.value.flatMap((x) => x?.value ?? [])
|
||||
)
|
||||
|
||||
watch(
|
||||
() => [...inspectorRefs.flatMap((x) => x!.value)],
|
||||
() => {
|
||||
this.tabs.value.set(
|
||||
this.restTab.currentActiveTab.value.id,
|
||||
activeInspections.value
|
||||
)
|
||||
},
|
||||
{ immediate: true }
|
||||
)
|
||||
},
|
||||
{ immediate: true, flush: "pre" }
|
||||
)
|
||||
this.watcherStopHandle = watch(
|
||||
() => [...activeInspections.value],
|
||||
() => {
|
||||
this.tabs.value.set(
|
||||
this.restTab.currentActiveTab.value.id,
|
||||
activeInspections.value
|
||||
)
|
||||
},
|
||||
{ immediate: true, flush: "pre" }
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
public deleteTabInspectorResult(tabID: string) {
|
||||
|
|
|
|||
|
|
@ -108,7 +108,7 @@ export class EnvironmentInspectorService extends Service implements Inspector {
|
|||
...collectionVariables,
|
||||
...this.aggregateEnvsWithValue.value,
|
||||
]
|
||||
const envKeys = environmentVariables.map((e) => e.key)
|
||||
const envKeysSet = new Set(environmentVariables.map((e) => e.key))
|
||||
|
||||
// Scan each string for <<VAR>> patterns
|
||||
target.forEach((element, index) => {
|
||||
|
|
@ -129,8 +129,7 @@ export class EnvironmentInspectorService extends Service implements Inspector {
|
|||
key: element,
|
||||
}
|
||||
|
||||
// If the variable doesn't exist, add an inspection
|
||||
if (!envKeys.includes(formattedExEnv)) {
|
||||
if (!envKeysSet.has(formattedExEnv)) {
|
||||
newErrors.push({
|
||||
id: `environment-not-found-${newErrors.length}`,
|
||||
text: {
|
||||
|
|
|
|||
|
|
@ -1,11 +1,11 @@
|
|||
import { Cookie, HoppRESTRequest } from "@hoppscotch/data"
|
||||
import { FaradayCage } from "faraday-cage"
|
||||
import { pipe } from "fp-ts/function"
|
||||
import * as TE from "fp-ts/lib/TaskEither"
|
||||
import { cloneDeep } from "lodash"
|
||||
|
||||
import { defaultModules, preRequestModule } from "~/cage-modules"
|
||||
import { HoppFetchHook, SandboxPreRequestResult, TestResult } from "~/types"
|
||||
import { acquireCage } from "~/utils/cage"
|
||||
|
||||
export const runPreRequestScriptWithFaradayCage = (
|
||||
preRequestScript: string,
|
||||
|
|
@ -21,7 +21,7 @@ export const runPreRequestScriptWithFaradayCage = (
|
|||
let finalRequest = request
|
||||
let finalCookies = cookies
|
||||
|
||||
const cage = await FaradayCage.create()
|
||||
const cage = await acquireCage()
|
||||
|
||||
try {
|
||||
const captureHook: { capture?: () => void } = {}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
import { HoppRESTRequest } from "@hoppscotch/data"
|
||||
import { FaradayCage } from "faraday-cage"
|
||||
import * as TE from "fp-ts/TaskEither"
|
||||
import { pipe } from "fp-ts/function"
|
||||
import { cloneDeep } from "lodash"
|
||||
|
|
@ -11,6 +10,7 @@ import {
|
|||
TestResponse,
|
||||
TestResult,
|
||||
} from "~/types"
|
||||
import { acquireCage } from "~/utils/cage"
|
||||
|
||||
export const runPostRequestScriptWithFaradayCage = (
|
||||
testScript: string,
|
||||
|
|
@ -30,7 +30,7 @@ export const runPostRequestScriptWithFaradayCage = (
|
|||
let finalTestResults = testRunStack
|
||||
const testPromises: Promise<void>[] = []
|
||||
|
||||
const cage = await FaradayCage.create()
|
||||
const cage = await acquireCage()
|
||||
|
||||
// Wrap entire execution in try-catch to handle QuickJS GC errors that can occur at any point
|
||||
try {
|
||||
|
|
|
|||
26
packages/hoppscotch-js-sandbox/src/utils/cage.ts
Normal file
26
packages/hoppscotch-js-sandbox/src/utils/cage.ts
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
import { FaradayCage } from "faraday-cage"
|
||||
|
||||
// Cached cage instance to avoid repeated WASM module allocations.
|
||||
let cachedCage: FaradayCage | null = null
|
||||
|
||||
// Detect if running in a test environment
|
||||
const isTestEnvironment =
|
||||
typeof process !== "undefined" && process.env.VITEST === "true"
|
||||
|
||||
/**
|
||||
* Returns a FaradayCage instance, creating and caching it on first access.
|
||||
* In test environments, always creates a fresh cage to avoid QuickJS GC corruption.
|
||||
*/
|
||||
export const acquireCage = async (): Promise<FaradayCage> => {
|
||||
// In test environments, create a fresh cage to avoid GC corruption
|
||||
if (isTestEnvironment) {
|
||||
return FaradayCage.create()
|
||||
}
|
||||
|
||||
// In production, cache the cage for performance
|
||||
if (!cachedCage) {
|
||||
cachedCage = await FaradayCage.create()
|
||||
}
|
||||
|
||||
return cachedCage
|
||||
}
|
||||
|
|
@ -1,4 +1,3 @@
|
|||
import { FaradayCage } from "faraday-cage"
|
||||
import { ConsoleEntry } from "faraday-cage/modules"
|
||||
import * as E from "fp-ts/Either"
|
||||
import { cloneDeep } from "lodash"
|
||||
|
|
@ -10,6 +9,7 @@ import {
|
|||
} from "~/types"
|
||||
|
||||
import { defaultModules, preRequestModule } from "~/cage-modules"
|
||||
import { acquireCage } from "~/utils/cage"
|
||||
|
||||
import { Cookie, HoppRESTRequest } from "@hoppscotch/data"
|
||||
import Worker from "./worker?worker&inline"
|
||||
|
|
@ -47,7 +47,7 @@ const runPreRequestScriptWithFaradayCage = async (
|
|||
let finalRequest = request
|
||||
let finalCookies = cookies
|
||||
|
||||
const cage = await FaradayCage.create()
|
||||
const cage = await acquireCage()
|
||||
|
||||
try {
|
||||
// Create a hook object to receive the capture function from the module
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
import { FaradayCage } from "faraday-cage"
|
||||
import { ConsoleEntry } from "faraday-cage/modules"
|
||||
import * as E from "fp-ts/Either"
|
||||
import { cloneDeep } from "lodash-es"
|
||||
|
|
@ -12,6 +11,7 @@ import {
|
|||
TestResponse,
|
||||
TestResult,
|
||||
} from "~/types"
|
||||
import { acquireCage } from "~/utils/cage"
|
||||
import { preventCyclicObjects } from "~/utils/shared"
|
||||
|
||||
import { Cookie, HoppRESTRequest } from "@hoppscotch/data"
|
||||
|
|
@ -57,7 +57,7 @@ const runPostRequestScriptWithFaradayCage = async (
|
|||
let finalCookies = cookies
|
||||
const testPromises: Promise<void>[] = []
|
||||
|
||||
const cage = await FaradayCage.create()
|
||||
const cage = await acquireCage()
|
||||
|
||||
try {
|
||||
// Create a hook object to receive the capture function from the module
|
||||
|
|
|
|||
Loading…
Reference in a new issue