Skip to content

Is Value.Free() after Value.Await() expected to be unsafe? #39

@cryguy

Description

@cryguy

Disclosure: This bug report was drafted with AI assistance (Claude). The reproduction code, stack traces, and root cause analysis have been manually verified. Happy to provide any additional information or adjust anything if I've misunderstood the intended API usage.

Summary

Calling promise.Free() after promise.Await() causes a WASM out-of-bounds memory access panic during rt.Close(). The crash affects all resolved value types (objects, strings, numbers) and occurs regardless of how the Promise was obtained (via GetPropertyStr or InvokeJS).

qjs version: v0.0.6
Go version: 1.23
OS: Linux (WSL2 on Windows 11), also reproduced on native Linux

Observed behavior

After calling Await() on a Promise, the Promise's underlying WASM handle appears to become invalid. Calling Free() on it afterward corrupts the WASM heap, which causes a panic when the runtime is closed:

failed to call QJS_Free: wasm error: out of bounds memory access

Notably:

  • Freeing a Promise without calling Await() first works fine (no crash).
  • Freeing only the resolved value (from Await()'s return) works fine.
  • The crash happens specifically when Free() is called on the Promise itself after Await().

This suggests js_std_await may internally consume the Promise's reference, and the Go Value handle isn't updated to reflect that.

Reproduction

The issue reproduces with both GetPropertyStr-obtained Promises and InvokeJS-returned Promises, ruling out the "don't free object properties directly" guidance as a factor.

Minimal go test reproduction (no dependencies beyond github.com/fastschema/qjs):

go.mod
module qjs-promise-bug

go 1.23

require github.com/fastschema/qjs v0.0.6
main_test.go — GetPropertyStr path
package main

import (
	"testing"

	"github.com/fastschema/qjs"
)

func catchPanic(fn func()) (panicVal any) {
	defer func() { panicVal = recover() }()
	fn()
	return nil
}

// evalPromise wraps a call in an object literal to capture the raw Promise
// before QuickJS auto-unwraps it (same pattern as value_test.go).
func evalPromise(t *testing.T, ctx *qjs.Context, call string) (wrapper, promise, resolved *qjs.Value) {
	t.Helper()
	var err error

	wrapper, err = ctx.Eval("test.js", qjs.Code(`({promise: `+call+`})`))
	if err != nil {
		t.Fatal(err)
	}

	promise = wrapper.GetPropertyStr("promise")
	if !promise.IsPromise() {
		t.Fatalf("expected Promise, got %s = %s", promise.Type(), promise.String())
	}

	resolved, err = promise.Await()
	if err != nil {
		t.Fatal(err)
	}
	return
}

// Test 1: promise.Free() after Await() crashes.
func TestPromiseFree_AfterAwait_Crashes(t *testing.T) {
	r := catchPanic(func() {
		rt, err := qjs.New()
		if err != nil {
			t.Fatal(err)
		}
		defer rt.Close()

		ctx := rt.Context()
		ctx.SetAsyncFunc("fetchData", func(this *qjs.This) {
			c := this.Context()
			obj := c.NewObject()
			obj.SetPropertyStr("status", c.NewInt32(200))
			obj.SetPropertyStr("body", c.NewString(`{"ok":true}`))
			this.Promise().Resolve(obj)
		})

		wrapper, promise, resolved := evalPromise(t, ctx, "fetchData()")
		defer wrapper.Free()

		t.Logf("resolved: status=%d", resolved.GetPropertyStr("status").Int32())

		promise.Free()  // <-- corrupts WASM heap
		resolved.Free()
	})

	if r != nil {
		t.Logf("BUG: rt.Close() panicked after promise.Free(): %v", r)
	}
}

// Test 2: Skipping promise.Free() avoids the crash.
func TestPromiseFree_SkipFree_OK(t *testing.T) {
	r := catchPanic(func() {
		rt, err := qjs.New()
		if err != nil {
			t.Fatal(err)
		}
		defer rt.Close()

		ctx := rt.Context()
		ctx.SetAsyncFunc("fetchData", func(this *qjs.This) {
			c := this.Context()
			obj := c.NewObject()
			obj.SetPropertyStr("status", c.NewInt32(200))
			this.Promise().Resolve(obj)
		})

		wrapper, _, resolved := evalPromise(t, ctx, "fetchData()")
		defer wrapper.Free()

		resolved.Free() // safe
		// Intentionally skip promise.Free()
	})

	if r != nil {
		t.Errorf("UNEXPECTED panic with workaround: %v", r)
	}
}

// Test 3: Scalar values also crash (not object-specific).
func TestPromiseFree_ScalarValue_AlsoCrashes(t *testing.T) {
	r := catchPanic(func() {
		rt, err := qjs.New()
		if err != nil {
			t.Fatal(err)
		}
		defer rt.Close()

		ctx := rt.Context()
		ctx.SetAsyncFunc("fetchString", func(this *qjs.This) {
			this.Promise().Resolve(this.Context().NewString("hello"))
		})

		wrapper, promise, resolved := evalPromise(t, ctx, "fetchString()")
		defer wrapper.Free()

		promise.Free()  // <-- also crashes for scalar values
		resolved.Free()
	})

	if r != nil {
		t.Logf("BUG (scalar): rt.Close() panicked: %v", r)
	}
}
invoke_test.go — InvokeJS path (no GetPropertyStr involved)

This test rules out "don't free properties directly" as a factor. The Promise is a direct return value from InvokeJS, not a property of another object.

package main

import (
	"testing"

	"github.com/fastschema/qjs"
)

func TestInvokeJS_PromiseFree_AfterAwait(t *testing.T) {
	r := catchPanic(func() {
		rt, err := qjs.New()
		if err != nil {
			t.Fatal(err)
		}
		defer rt.Close()

		ctx := rt.Context()
		ctx.SetAsyncFunc("fetch", func(this *qjs.This) {
			c := this.Context()
			obj := c.NewObject()
			obj.SetPropertyStr("status", c.NewInt32(200))
			this.Promise().Resolve(obj)
		})

		_, err = ctx.Eval("setup.js", qjs.Code(`
			globalThis.worker = {
				handle: async function() { return await fetch(); }
			};
		`))
		if err != nil {
			t.Fatal(err)
		}

		worker := ctx.Global().GetPropertyStr("worker")
		result, err := worker.InvokeJS("handle")
		worker.Free()
		if err != nil {
			t.Fatal(err)
		}

		if result.IsPromise() {
			awaited, err := result.Await()
			if err != nil {
				t.Fatal(err)
			}

			result.Free()  // <-- crashes: same bug via InvokeJS path
			awaited.Free()
		}
	})

	if r != nil {
		t.Logf("CRASHED (InvokeJS return value, not a property): %v", r)
	}
}

Run

go mod tidy
go test -v -count=1

Output

=== RUN   TestPromiseFree_AfterAwait_Crashes
    resolved: status=200
    BUG: rt.Close() panicked after promise.Free():
        failed to call QJS_Free: wasm error: out of bounds memory access
--- PASS

=== RUN   TestPromiseFree_SkipFree_OK
--- PASS

=== RUN   TestPromiseFree_ScalarValue_AlsoCrashes
    BUG (scalar): rt.Close() panicked:
        failed to call QJS_Free: wasm error: out of bounds memory access
--- PASS

=== RUN   TestInvokeJS_PromiseFree_AfterAwait
    CRASHED (InvokeJS return value, not a property):
        failed to free QJS runtime: wasm error: out of bounds memory access
--- PASS

Why this is confusing as a user

The README's "Awaiting a promise" example follows a pattern that happens to avoid this issue:

mainFunc := result.GetPropertyStr("main")
val, err := mainFunc.Await()
log.Println("Awaited value:", val.String())
// mainFunc.Free() is not called here

The example doesn't call Free() on mainFunc (the promise) after Await(), which avoids the crash. But the general guidance says "Always call result.Free() on JavaScript values," so it's natural to assume Free() should be called on all values, including awaited Promises.

It would be helpful to either:

  • Document that Await() consumes the Promise (so Free() should not be called afterward), or
  • Make Free() safe to call after Await().

Current workaround

Skip promise.Free() after Await() and free only the resolved value:

resolved, err := promise.Await()
// ...
resolved.Free()   // safe
// promise.Free()  // skip this — causes heap corruption

This leaks the Promise's WASM-side memory but avoids the crash.

Possible fix directions

A few options (not prescriptive — you know the codebase best):

  1. Invalidate the handle in Await() so that subsequent Free() is a no-op:

    func (v *Value) Await() (*Value, error) {
        // ...
        result := v.Call("js_std_await", v.Ctx(), v.Raw())
        v.handle = nil  // mark as consumed
        return normalizeJsValue(v.context, result)
    }
  2. Dup the refcount before js_std_await so both the internal free and the user's Free() are valid.

  3. Make Free() idempotent with a stale-handle guard.

Context

We discovered this while building a worker runtime that uses SetAsyncFunc for a fetch() polyfill. The Promise was returned via InvokeJS on an async JS method, awaited in Go, and freed as part of normal cleanup. The crash was intermittent in production because it only corrupted the heap — the actual panic surfaced later during runtime teardown or on the next request in a pooled runtime.

Thank you for building qjs — it's a great project and the WASM/Wazero approach is really clever. Happy to help test a fix if useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions