Skip to content

refactor(js): centralize runtime session handling - #7449

Merged
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/refactor/js/centralize-runtime-session-handling
Jun 8, 2026
Merged

refactor(js): centralize runtime session handling#7449
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/refactor/js/centralize-runtime-session-handling

Conversation

@dwisiswant0

@dwisiswant0 dwisiswant0 commented Jun 8, 2026

Copy link
Copy Markdown
Member

Proposed changes

Move the shared goja execution lifecycle out of
the pooled and non-pooled paths into a common
session helper.

Per-path hooks now handle setup, callback
registration, interrupt waiting, cleanup, runtime
return, and slot release, while abandon handling
stays centralized. Abandoned runtimes are no
longer touched or reused until their orphaned
goroutine exits.

Closes #7417

Proof

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Tests

    • Significantly expanded test coverage for JavaScript runtime execution, including error recovery, timeout handling, and safe reuse of runtimes across multiple executions.
  • Bug Fixes

    • Improved runtime resource cleanup when callbacks error or execution times out, ensuring proper state reset and allowing runtimes to be reused safely.

Move the shared goja execution lifecycle out of
the pooled and non-pooled paths into a common
session helper.

Per-path hooks now handle setup, callback
registration, interrupt waiting, cleanup, runtime
return, and slot release, while abandon handling
stays centralized. Abandoned runtimes are no
longer touched or reused until their orphaned
goroutine exits.

Closes #7417

Signed-off-by: Dwi Siswanto <git@dw1.io>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b79a664c-93bd-4589-8ed1-a80bd82baf3d

📥 Commits

Reviewing files that changed from the base of the PR and between 20cbf0b and 108c350.

📒 Files selected for processing (4)
  • pkg/js/compiler/compiler_test.go
  • pkg/js/compiler/non-pool.go
  • pkg/js/compiler/pool.go
  • pkg/js/compiler/session.go

Walkthrough

This PR refactors the JavaScript runtime lifecycle by introducing a session abstraction that centralizes ownership of runtime setup, execution, interruption, cleanup, and slot management. Both pooled and non-pooled execution paths now delegate to the session rather than handling these concerns separately.

Changes

Runtime Session Lifecycle Refactoring

Layer / File(s) Summary
Session abstraction and lifecycle management
pkg/js/compiler/session.go
Defines session state machine (running, completed, abandoned) with configurable hooks for runtime preparation, cleanup, result finalization, and resource release. Handles async execution, context cancellation by interrupting the runtime and marking the session abandoned, and ensures cleanup runs at most once per category. Abandoned sessions skip finalization and normal release, optionally launching a reaper goroutine to defer slot release until the execution goroutine exits.
Non-pooled execution adoption
pkg/js/compiler/non-pool.go
Replaces conditional errors.Is(errRuntimeTerminationTimeout) slot-ownership logic with unified session initialization. Both normal and abandoned slot releases route through the session's ephemeral Done callbacks.
Pooled execution adoption
pkg/js/compiler/pool.go
Replaces direct runtime handling with session setup that wires: export helper registration (prepare), export global cleanup (cleanup), result finalization (buffer assembly), and runtime return-to-pool. Introduces registerExportHelpers to register Export and ExportAs globals. Execution routes to pooled or non-pooled based on whether Export/ExportAs tokens are present in source.
Comprehensive test coverage
pkg/js/compiler/compiler_test.go
Adds 9 new tests and a useRuntimePool helper. Tests validate callback error cleanup, prompt interruption with safe reuse, non-pooled slot release, non-pooled abandonment when callbacks block (slot held until orphan exits), pooled state reset before reuse, pooled callback errors, session abandonment skip semantics (no finalize/release hooks), pooled abandonment prevents caller cleanup and reuse, and abandoned runtimes are not returned to the pool.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • projectdiscovery/nuclei#7378: Both PRs address pooled-runtime abandonment behavior when interrupt grace period elapses, particularly delaying slot release until the orphan exits and preventing reuse; the main PR refactors this into the session abstraction.

Suggested labels

Type: Bug

Suggested reviewers

  • dogancanbakir

Poem

🐰 A session is born, to shepherd each runtime,
No more scattered cleanup, no tangled decline,
When context times out, it abandons with grace,
Orphans can linger in their own space,
One owner, one path—what elegance, divine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: centralizing runtime session handling in the JavaScript execution subsystem.
Linked Issues check ✅ Passed The PR successfully implements all coding objectives from #7417: centralizes session lifecycle into a new session.go abstraction, ensures single runtime ownership per goroutine, implements abandonment handling with reaper delegation, and refactors pooled/non-pooled paths to use the common session layer.
Out of Scope Changes check ✅ Passed All changes are directly scoped to runtime session lifecycle refactoring: new session.go abstraction, updates to non-pool.go and pool.go to use session layer, and comprehensive test coverage of the new behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dwisiswant0/refactor/js/centralize-runtime-session-handling

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Command failed


Comment @coderabbitai help to get the list of available commands and usage tips.

@dogancanbakir
dogancanbakir requested a review from Mzack9999 June 8, 2026 10:16
@Mzack9999
Mzack9999 merged commit f44027e into dev Jun 8, 2026
19 checks passed
@Mzack9999
Mzack9999 deleted the dwisiswant0/refactor/js/centralize-runtime-session-handling branch June 8, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MAINTENANCE] Refactor javascript-protocol runtime lifecycle

2 participants