Skip to content

[WIP] Implement owner-goroutine check for QuickJS Runtime/Context#727

Closed
Copilot wants to merge 1 commit into
mainfrom
copilot/add-owner-goroutine-check
Closed

[WIP] Implement owner-goroutine check for QuickJS Runtime/Context#727
Copilot wants to merge 1 commit into
mainfrom
copilot/add-owner-goroutine-check

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 4, 2026

  • Understand existing codebase structure
  • Create internal/goid package with Get() int64 using runtime.Stack() parsing (portable, Go 1.21-1.26 safe)
  • Add ownerGID int64 field to Runtime struct, set in NewRuntime()
  • Add ownerGID int64 field to Context struct, set in NewContext()
  • Add checkOwner() helper to both Runtime and Context (panics with clear message on goroutine mismatch)
  • Hook checkOwner() into Context.hasValidRef(), Context.Close(), Context.ProcessJobs(), Context.Globals(), Context.NewString(), Context.Eval()
  • Hook checkOwner() into Runtime.NewContext() and Runtime.Close()
  • Create owner_test.go with tests verifying the owner-goroutine check panics from a non-owner goroutine and passes from the owner goroutine
  • Update README.md and README_zh-cn.md to document the owner-goroutine enforcement
Original prompt

Repo: buke/quickjs-go

Goal

  • Implement an "owner-goroutine check" for QuickJS Runtime/Context usage.
  • Must be compatible with Go 1.26 and support amd64/arm64.

Background

  • README states: "For a given Runtime and its Contexts, create, use, and close them from one serialized owner goroutine" and "Goroutines must NOT call QuickJS/Context APIs directly; always schedule back to the Context thread via ctx.Schedule".
  • Currently the code uses ctx.Schedule and ctx.ProcessJobs but does not enforce owner goroutine affinity.

Requirements

  1. Add a mechanism to record an "owner" at Runtime and/or Context creation time (e.g., goroutine ID or equivalent) and enforce it when calling QuickJS / Context APIs directly.
  2. Provide a fast, cross-arch implementation.
    • Prefer using runtime-internal g pointer access if feasible, but avoid fragile offsets if possible.
    • If a stable, safe method isn't possible, implement a best-effort approach with build tags and a safe fallback (e.g., parsing runtime.Stack) and document the trade-offs.
  3. The check should fail fast with a clear error/panic message when QuickJS APIs are called from a non-owner goroutine.
    • Decide how to handle this across the API surface: return exception Value, return nil, or panic; be consistent.
  4. Ensure calls made from ctx.Schedule job execution path are considered "owner" (since those jobs are run on the context thread).
    • If Schedule jobs can be executed by any goroutine calling ProcessJobs/Await/Loop, decide what the owner is and enforce consistently.
  5. Tests:
    • Add unit tests verifying that calling e.g. ctx.Eval / ctx.NewString / ctx.Globals / ctx.Close from a different goroutine triggers the owner check.
    • Add tests that show ctx.Schedule + ctx.Await/ProcessJobs on the owner goroutine works.
    • Ensure tests pass on amd64/arm64.

Implementation guidance

  • Investigate where the best place is to store and validate owner state:
    • Runtime.NewRuntime / Runtime.NewContext / Context.initScheduler / Context.Schedule / Context.ProcessJobs / Context.Await / Context.Loop.
  • Consider adding internal helper(s) like:
    • func (ctx *Context) assertOwner()
    • func (r *Runtime) assertOwner()
    • and an internal package (e.g., internal/goid) with build-tagged implementations.
  • Keep public API changes minimal.
  • Update README (English + zh-cn) to describe the behavior and any opt-out (if added).

Notes

  • If strict goroutine-id enforcement is too risky, consider an alternative: record owner OS thread (using runtime.LockOSThread requirement) or use a dedicated goroutine running the event loop; but user asked specifically for owner-goroutine check.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: assembly goid

Assistant: In Go assembly, goid usually means the current goroutine’s ID—but there is no supported, stable Go API or ABI to read it directly from assembly.

What you can do depends on your goal:

1) If you just need a per-goroutine value: use TLS / g pointer (assembly)

On most architectures the runtime keeps a register pointing to the current g (goroutine) struct (often referred to as g / g_m / “G pointer”). From assembly you can often access fields of g, but the layout and offsets are runtime-internal and version-dependent (can break across Go versions).

Typical pattern (conceptually):

  • get current g pointer (arch-specific)
  • load g.goid field (offset varies)
  • return it

To do this safely you must generate offsets from the exact Go version you build with (people use go tool compile -S, runtime headers, or go:linkname-based hacks). It’s not portable.

2) If you need it in Go code: don’t (preferred)

Go intentionally doesn’t expose goroutine IDs. Usual alternatives:

  • pass an explicit request ID / context value
  • use runtime/pprof labels for tracing
  • use goid only for debugging (best-effort hacks)

3) If you want a “best effort” debugging hack

Some projects parse runtime.Stack() output ("goroutine 1234 [running]:") to extract an ID. This is slow and not guaranteed, but doesn’t rely on runtime struct offsets.


Clarifying questions (so I can give the exact assembly you need)

  1. Which architecture/OS? (amd64/linux, arm64/darwin, windows, etc.)
  2. Which Go version (exact, e.g. go1.22.5, go1.23.x, etc.)?
  3. Do you need the ID for debugging only, or are you building a feature that depends on it?

If you answer those 3, I can show the correct Go assembly skeleton for that arch (Plan 9 syntax), and explain where the g pointer comes from and how to wire it to a Go wrapper.

User: 推荐一些 current goroutine’s ID 的库吧

Assistant: 下面这些库都能拿到“当前 goroutine 的 ID(非官方、仅供调试/观测)”。提醒一句:Go 官方明确不保证 goroutine id 的可用性/稳定性,很多实现要么读 runtime 内部结构、要么解析 runtime.Stack,跨 Go 版本可能会碎。(stackoverflow.com)...

This pull request was created from Copilot chat.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (c1164c3) to head (2f420eb).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #727   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines         2872      2872           
=========================================
  Hits          2872      2872           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1164c3...2f420eb. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@buke buke closed this Apr 4, 2026
@buke buke deleted the copilot/add-owner-goroutine-check branch April 4, 2026 03:30
Copilot AI requested a review from buke April 4, 2026 03:30
Copilot stopped work on behalf of buke due to an error April 4, 2026 03:30
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.

2 participants