diff --git a/docs/reference/headless-linux-server.md b/docs/reference/headless-linux-server.md index 3eed97535c1..67db9309469 100644 --- a/docs/reference/headless-linux-server.md +++ b/docs/reference/headless-linux-server.md @@ -294,18 +294,18 @@ initial load is still running, or while it is waiting on a certificate decision. A navigation that has still not finished by the load timeout is deliberately reclaimable — otherwise one stalled page per create could hold a renderer forever, which is the failure this exists to prevent; waking it simply -retries the address. +retries the address. Parking never closes a page: the page stays open until +something closes it, exactly as on the desktop. Two evictors decide what stays resident, mirroring terminal pane parking. The cap is the primary one: -| Variable | Default | Meaning | -| ------------------------------------------ | -------- | ---------------------------------------------------------------------- | -| `ORCA_HEADLESS_BROWSER_RESIDENT_LIMIT` | `4` | Renderers kept resident. Least-recently-used pages park past this. | -| `ORCA_HEADLESS_BROWSER_PARK_IDLE_MS` | `300000` | A page parks after this long untouched, even under the cap. | -| `ORCA_HEADLESS_BROWSER_PARK_GRACE_MS` | `30000` | A page is never parked this soon after its last command. | -| `ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS` | `15000` | How often the server re-evaluates residency. | -| `ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES` | `100` | Open pages retained before the oldest parked ones are closed outright. | +| Variable | Default | Meaning | +| -------------------------------------- | -------- | ------------------------------------------------------------------ | +| `ORCA_HEADLESS_BROWSER_RESIDENT_LIMIT` | `4` | Renderers kept resident. Least-recently-used pages park past this. | +| `ORCA_HEADLESS_BROWSER_PARK_IDLE_MS` | `300000` | A page parks after this long untouched, even under the cap. | +| `ORCA_HEADLESS_BROWSER_PARK_GRACE_MS` | `30000` | A page is never parked this soon after its last command. | +| `ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS` | `15000` | How often the server re-evaluates residency. | Raise the limit on a host with memory to spare, or set both the limit and the idle window very high to keep every page resident — at the cost this diff --git a/src/main/browser/browser-manager.ts b/src/main/browser/browser-manager.ts index 0932ead11ac..427358cbe59 100644 --- a/src/main/browser/browser-manager.ts +++ b/src/main/browser/browser-manager.ts @@ -201,8 +201,6 @@ type ActiveDownload = { savePath: string reservationKey: string | null receivedBytes: number - /** Last time Electron reported progress; the liveness signal a reclaimer needs. */ - lastProgressAt: number transientState: BrowserDownloadProgressEvent['state'] terminalEvent: BrowserDownloadFinishedEvent | null startedSent: boolean @@ -1474,23 +1472,18 @@ export class BrowserManager { : (this.loadErrorsByGuestId.get(webContentsId) ?? null) } - /** - * When this page's in-flight downloads last made progress, or null when it has - * none. Why (STA-4341): unregisterGuest cancels a tab's in-flight downloads, so - * a reclaimer must not take a page that is still writing one — the desktop guest - * budget vetoes eviction for the same reason. A timestamp rather than a boolean - * because a download that stalls forever must not pin a renderer forever; the - * caller owns that bound. - */ - getBrowserPageActiveDownloadProgressAt(browserPageId: string): number | null { - let latest: number | null = null + // Why (STA-4341): unregisterGuest cancels a tab's in-flight downloads, so a + // reclaimer must not take a page that is still writing one. Mirrors the + // desktop guest budget's veto (browser-page-download-activity.ts) exactly, + // including its unbounded nature: a download is treated as work in progress + // for as long as it stays active. + hasActiveBrowserPageDownload(browserPageId: string): boolean { for (const download of this.downloadsById.values()) { if (download.browserTabId === browserPageId && !download.terminalEvent) { - latest = - latest === null ? download.lastProgressAt : Math.max(latest, download.lastProgressAt) + return true } } - return latest + return false } getBrowserPageCertificateFailure(browserPageId: string): BrowserCertificateFailure | null { @@ -1638,7 +1631,6 @@ export class BrowserManager { savePath: fallbackSavePath, reservationKey: destination?.reservationKey ?? null, receivedBytes: 0, - lastProgressAt: Date.now(), transientState: null, terminalEvent: null, startedSent: false, @@ -1680,7 +1672,6 @@ export class BrowserManager { const updatedHandler = (_event: Electron.Event, state: 'progressing' | 'interrupted'): void => { download.receivedBytes = this.getDownloadReceivedBytes(download.item) - download.lastProgressAt = Date.now() download.transientState = state this.sendDownloadProgress(download.browserTabId, { browserPageId: download.browserTabId ?? undefined, diff --git a/src/main/browser/offscreen-browser-backend-reclaim.test.ts b/src/main/browser/offscreen-browser-backend-reclaim.test.ts index 920dd7135fa..d90a61e097c 100644 --- a/src/main/browser/offscreen-browser-backend-reclaim.test.ts +++ b/src/main/browser/offscreen-browser-backend-reclaim.test.ts @@ -96,7 +96,6 @@ type Harness = { pagesChanged: (string | undefined)[] certificateFailurePageIds: Set downloadingPageIds: Set - downloadProgressAt: number } function createHarness( @@ -112,7 +111,6 @@ function createHarness( activePageId: overrides.activePageId, certificateFailurePageIds: new Set(overrides.certificateFailurePageIds ?? []), downloadingPageIds: new Set(overrides.downloadingPageIds ?? []), - downloadProgressAt: 1_000_000, pagesChanged: [] as (string | undefined)[] } const order: string[] = [] @@ -145,8 +143,8 @@ function createHarness( getBrowserPageLoadError: () => overrides.loadError ?? null, getBrowserPageCertificateFailure: (browserPageId: string) => state.certificateFailurePageIds.has(browserPageId) ? { challengeId: 'c' } : null, - getBrowserPageActiveDownloadProgressAt: (browserPageId: string) => - state.downloadingPageIds.has(browserPageId) ? state.downloadProgressAt : null + hasActiveBrowserPageDownload: (browserPageId: string) => + state.downloadingPageIds.has(browserPageId) } as unknown as BrowserManager const bridge = { @@ -181,10 +179,7 @@ function createHarness( return state.pagesChanged }, certificateFailurePageIds: state.certificateFailurePageIds, - downloadingPageIds: state.downloadingPageIds, - set downloadProgressAt(value: number) { - state.downloadProgressAt = value - } + downloadingPageIds: state.downloadingPageIds } } @@ -195,7 +190,6 @@ beforeEach(() => { process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = '60000' process.env.ORCA_HEADLESS_BROWSER_PARK_GRACE_MS = '5000' process.env.ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS = '100000' - process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '100' }) describe('OffscreenBrowserBackend reclamation', () => { @@ -330,43 +324,6 @@ describe('OffscreenBrowserBackend reclamation', () => { expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['a']) }) - it('does not close a parked page that was woken while an earlier close was in flight', async () => { - let releaseFirstTeardown = (): void => {} - process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '2' - // Why: pinning the two newer pages keeps the park loop empty, so the sweep - // that blocks is the retention close — the path under test. - const h = createHarness({ pinned: new Set(['c', 'd']) }) - const bridge = h.bridge as unknown as { onTabClosed: ReturnType } - - for (const id of ['a', 'b']) { - await h.backend.createTab({ url: `https://${id}`, browserPageId: id }) - h.clock.value += 1_000 - } - h.clock.value += 120_000 - await h.backend.reclaimIdlePages() - expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['a', 'b']) - - // Two more push the retention budget over, dooming the two oldest parked. - for (const id of ['c', 'd']) { - await h.backend.createTab({ url: `https://${id}`, browserPageId: id }) - h.clock.value += 1_000 - } - h.clock.value += 120_000 - bridge.onTabClosed.mockImplementationOnce( - async () => new Promise((resolve) => (releaseFirstTeardown = resolve)) - ) - - const sweep = h.backend.reclaimIdlePages() - await Promise.resolve() - await h.backend.wakeTab('b') - releaseFirstTeardown() - await sweep - - // a was closed; b survived the stale close list and is resident again. - expect(await h.backend.wakeTab('a')).toBe(false) - expect(await h.backend.wakeTab('b')).toBe(true) - }) - it('does not park a page whose wake is still rebuilding it', async () => { // Why: a wake is resident but not yet loading while it awaits the process // swap, so without an explicit pin the sweep can destroy it mid-rebuild. @@ -428,14 +385,13 @@ describe('OffscreenBrowserBackend reclamation', () => { expect(h.pagesChanged).toEqual(['wt-1']) }) - it('does not park a page whose download is still advancing', async () => { + it('does not park a page that is still writing a download', async () => { // Why: releasing a renderer unregisters its guest, and browser-manager // cancels that page's in-flight downloads on unregister. The desktop guest // budget vetoes eviction for the same reason. const h = createHarness({ downloadingPageIds: ['a'] }) await h.backend.createTab({ url: 'https://a', browserPageId: 'a' }) h.clock.value += 120_000 - h.downloadProgressAt = h.clock.value expect(await h.backend.reclaimIdlePages()).toEqual([]) @@ -443,18 +399,6 @@ describe('OffscreenBrowserBackend reclamation', () => { expect(await h.backend.reclaimIdlePages()).toEqual(['a']) }) - it('parks a page whose download has stalled so the cap cannot be defeated', async () => { - // Why: a download that never progresses again would otherwise pin its - // renderer forever, and one stalled download per page would defeat the - // resident cap outright — the same shape as the load-pin bug before it. - const h = createHarness({ downloadingPageIds: ['a'] }) - await h.backend.createTab({ url: 'https://a', browserPageId: 'a' }) - h.downloadProgressAt = h.clock.value - h.clock.value += 120_000 - - expect(await h.backend.reclaimIdlePages()).toEqual(['a']) - }) - it('does not park a page waiting on a certificate decision', async () => { // Why: the challenge id dies with the renderer, so parking would discard // both the warning and the ability to approve it. @@ -824,21 +768,6 @@ describe('OffscreenBrowserBackend reclamation', () => { expect(await h.backend.wakeTab('a')).toBe(false) }) - it('closes the oldest parked pages once too many records are retained', async () => { - // Why: parking bounds renderer processes, not the page records behind them. - process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '2' - const h = createHarness() - for (const id of ['a', 'b', 'c', 'd']) { - await h.backend.createTab({ url: `https://example.test/${id}`, browserPageId: id }) - h.clock.value += 1_000 - } - h.clock.value += 120_000 - await h.backend.reclaimIdlePages() - - expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['c', 'd']) - expect(h.windows.every((win) => win.isDestroyed())).toBe(true) - }) - it('scopes parked listings and implicit targeting to a worktree', async () => { const h = createHarness() await h.backend.createTab({ url: 'https://a', browserPageId: 'a', worktreeId: 'wt-1' }) diff --git a/src/main/browser/offscreen-browser-backend.ts b/src/main/browser/offscreen-browser-backend.ts index d7e1359aa11..7527879714c 100644 --- a/src/main/browser/offscreen-browser-backend.ts +++ b/src/main/browser/offscreen-browser-backend.ts @@ -48,8 +48,8 @@ export class OffscreenBrowserBackend implements BrowserBackend { isWaking: (browserPageId) => this.waking.has(browserPageId), hasCertificateChallenge: (browserPageId) => this.browserManager.getBrowserPageCertificateFailure(browserPageId) !== null, - activeDownloadProgressAt: (browserPageId) => - this.browserManager.getBrowserPageActiveDownloadProgressAt(browserPageId), + hasActiveDownload: (browserPageId) => + this.browserManager.hasActiveBrowserPageDownload(browserPageId), isHostPinned: (browserPageId) => this.options.isPagePinned?.(browserPageId) === true, park: (browserPageId) => this.parkPage(browserPageId), close: (browserPageId) => this.closeTab(browserPageId), diff --git a/src/main/browser/offscreen-browser-page-reclaim.test.ts b/src/main/browser/offscreen-browser-page-reclaim.test.ts index 110357290bb..17a83da58f5 100644 --- a/src/main/browser/offscreen-browser-page-reclaim.test.ts +++ b/src/main/browser/offscreen-browser-page-reclaim.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest' import { - selectOffscreenBrowserPagesToClose, selectOffscreenBrowserPagesToPark, type OffscreenBrowserReclaimCandidate, type OffscreenBrowserReclaimPolicy @@ -12,8 +11,7 @@ const POLICY: OffscreenBrowserReclaimPolicy = { residentLimit: 2, idleParkMs: 60_000, parkGraceMs: 5_000, - sweepIntervalMs: 1_000, - retainedPageLimit: 100 + sweepIntervalMs: 1_000 } function page( @@ -98,24 +96,3 @@ describe('selectOffscreenBrowserPagesToPark', () => { ).toEqual([]) }) }) - -describe('selectOffscreenBrowserPagesToClose', () => { - it('retains every parked page under the limit', () => { - expect(selectOffscreenBrowserPagesToClose([page('a', 1), page('b', 2)], 2, POLICY)).toEqual([]) - }) - - it('closes the least recently used parked pages down to the limit', () => { - // Why: parking bounds renderer processes, not the records behind them. - const parked = [page('new', 1_000), page('old', 90_000), page('older', 99_000)] - expect( - selectOffscreenBrowserPagesToClose(parked, 5, { ...POLICY, retainedPageLimit: 3 }) - ).toEqual(['older', 'old']) - }) - - it('never closes a pinned page', () => { - const parked = [page('pinned', 99_000, true), page('idle', 90_000)] - expect( - selectOffscreenBrowserPagesToClose(parked, 4, { ...POLICY, retainedPageLimit: 2 }) - ).toEqual(['idle']) - }) -}) diff --git a/src/main/browser/offscreen-browser-page-reclaim.ts b/src/main/browser/offscreen-browser-page-reclaim.ts index 77b7891c43f..3dd69682139 100644 --- a/src/main/browser/offscreen-browser-page-reclaim.ts +++ b/src/main/browser/offscreen-browser-page-reclaim.ts @@ -16,22 +16,12 @@ export const OFFSCREEN_BROWSER_PARK_GRACE_MS = 30_000 /** How often the backend re-evaluates which pages should be resident. */ export const OFFSCREEN_BROWSER_SWEEP_INTERVAL_MS = 15_000 -/** Parked page records retained before the oldest are closed outright. */ -export const OFFSCREEN_BROWSER_RETAINED_PAGE_LIMIT = 100 -/** - * How long a download keeps its page resident after its last progress. Parking - * cancels a page's in-flight downloads, so an advancing one must veto — but a - * download that stalls forever must not, or one stalled download per page would - * defeat the resident cap outright. - */ -export const OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS = 60_000 export type OffscreenBrowserReclaimPolicy = { residentLimit: number idleParkMs: number parkGraceMs: number sweepIntervalMs: number - retainedPageLimit: number } export type OffscreenBrowserReclaimCandidate = { @@ -67,13 +57,6 @@ export function readOffscreenBrowserReclaimPolicy(): OffscreenBrowserReclaimPoli sweepIntervalMs: Math.max( 100, readPositiveIntEnv('ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS', OFFSCREEN_BROWSER_SWEEP_INTERVAL_MS) - ), - retainedPageLimit: Math.max( - 1, - readPositiveIntEnv( - 'ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES', - OFFSCREEN_BROWSER_RETAINED_PAGE_LIMIT - ) ) } } @@ -123,30 +106,3 @@ export function selectOffscreenBrowserPagesToPark( return parkable.map((page) => page.browserPageId).filter((id) => parked.has(id)) } - -/** - * Pick parked pages to close outright. Parking bounds renderer processes but - * not the records behind them, so an agent that opens pages forever and closes - * none would still grow the backend without limit. Only parked pages are - * eligible and only the least recently used go, so this can never take a page - * an agent is still working with. - */ -export function selectOffscreenBrowserPagesToClose( - parkedPages: readonly OffscreenBrowserReclaimCandidate[], - totalPageCount: number, - policy: OffscreenBrowserReclaimPolicy -): string[] { - const excess = totalPageCount - policy.retainedPageLimit - if (excess <= 0) { - return [] - } - return parkedPages - .filter((page) => !page.pinned) - .sort( - (left, right) => - left.lastActivityAt - right.lastActivityAt || - left.browserPageId.localeCompare(right.browserPageId) - ) - .slice(0, excess) - .map((page) => page.browserPageId) -} diff --git a/src/main/browser/offscreen-browser-page-reclaimer.ts b/src/main/browser/offscreen-browser-page-reclaimer.ts index 581a5fb81c6..73a2cb44861 100644 --- a/src/main/browser/offscreen-browser-page-reclaimer.ts +++ b/src/main/browser/offscreen-browser-page-reclaimer.ts @@ -3,8 +3,6 @@ import type { OffscreenBrowserPage } from './offscreen-browser-open-pages' import { - OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS, - selectOffscreenBrowserPagesToClose, selectOffscreenBrowserPagesToPark, type OffscreenBrowserReclaimPolicy } from './offscreen-browser-page-reclaim' @@ -26,8 +24,8 @@ export type OffscreenBrowserReclaimerDeps = { isWaking: (browserPageId: string) => boolean /** A certificate challenge the page is blocked on. */ hasCertificateChallenge: (browserPageId: string) => boolean - /** When the page's in-flight downloads last advanced, null when it has none. */ - activeDownloadProgressAt: (browserPageId: string) => number | null + /** A download the page is still writing. */ + hasActiveDownload: (browserPageId: string) => boolean /** A client streaming the page, or a command in flight against it. */ isHostPinned: (browserPageId: string) => boolean park: (browserPageId: string) => Promise @@ -63,7 +61,6 @@ export class OffscreenBrowserPageReclaimer { await this.deps.park(browserPageId) parked.push(browserPageId) } - await this.closeOverRetainedPages() return parked } @@ -100,30 +97,6 @@ export class OffscreenBrowserPageReclaimer { this.sweepInFlight = null } - // Why: parking bounds renderer processes, not the records behind them. An - // agent that opens pages forever and closes none would still grow the backend - // without limit, so the oldest parked pages are eventually closed outright. - private async closeOverRetainedPages(): Promise { - const doomed = selectOffscreenBrowserPagesToClose( - this.deps.pages.parked().map((page) => this.toCandidate(page)), - this.deps.pages.size, - this.deps.policy - ) - for (const browserPageId of doomed) { - // Why: closing is destructive and awaits teardown, so a page selected - // here can be woken before its turn comes. Only close one that is still - // parked and still unwanted. - const page = this.deps.pages.get(browserPageId) - if (!page || page.window || !this.isSafeToReclaim(browserPageId)) { - continue - } - console.warn( - `[offscreen-browser] closing parked page ${browserPageId}: more than ${this.deps.policy.retainedPageLimit} pages are open` - ) - await this.deps.close(browserPageId) - } - } - // Why: a page is off limits while anything depends on its renderer — a client // streaming it, a command in flight, its first navigation still committing, a // wake still rebuilding it, a certificate decision it is blocked on, or a @@ -137,21 +110,14 @@ export class OffscreenBrowserPageReclaimer { // Why: a challenge id dies with the renderer, so parking would discard // both the warning and the ability to approve it. this.deps.hasCertificateChallenge(page.browserPageId) || - this.hasAdvancingDownload(page.browserPageId) || + // Why: releasing a renderer unregisters its guest, and that cancels the + // page's in-flight downloads. Mirrors the desktop guest budget's veto + // (browser-guest-worktree-retention.ts). + this.deps.hasActiveDownload(page.browserPageId) || this.deps.isHostPinned(page.browserPageId) ) } - // Why: releasing a renderer unregisters its guest, and that cancels the page's - // in-flight downloads — the desktop guest budget vetoes eviction for the same - // reason (browser-guest-worktree-retention.ts). Bounded by progress, because a - // download that stalls forever would otherwise pin its renderer forever, and - // one stalled download per page would defeat the resident cap outright. - private hasAdvancingDownload(browserPageId: string): boolean { - const progressAt = this.deps.activeDownloadProgressAt(browserPageId) - return progressAt !== null && this.deps.now() - progressAt < OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS - } - private toCandidate(page: OffscreenBrowserPage): { browserPageId: string lastActivityAt: number