Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add support for `Isolate.SetPromiseRejectedCallback` allowing the embedder to react to unhandled promise rejectsions, or when resolving a promise multiple times.

### Changed

## [v0.33.0] - 2025-05-15
Expand Down
21 changes: 20 additions & 1 deletion context.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include "context.h"
#include "deps/include/v8-external.h"
#include "deps/include/v8-template.h"

#include "context-macros.h"
Expand Down Expand Up @@ -27,11 +29,13 @@ ContextPtr NewContext(IsolatePtr iso,
// side to lookup the context in the context registry. We use slot 1 as slot 0
// has special meaning for the Chrome debugger.
Local<Context> local_ctx = Context::New(iso, nullptr, global_template);
local_ctx->SetEmbedderData(1, Integer::New(iso, ref));

m_ctx* ctx = new m_ctx;
ctx->ptr.Reset(iso, local_ctx);
ctx->iso = iso;
local_ctx->SetEmbedderData(ContextDataIndex::REF, Integer::New(iso, ref));
local_ctx->SetEmbedderData(ContextDataIndex::WRAPPER_PTR,
External::New(iso, ctx));
return ctx;
}

Expand All @@ -56,6 +60,15 @@ void ContextFree(ContextPtr ctx) {
delete ctx;
}

m_value* track_value(m_ctx* ctx, Local<Value> value) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to make a helper.

Shouldn't this be an m_value constructor (without tracked_value())? Seems RAII-worthy.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have had similar ideas, but something about cleanup I didn't think regarding freeing up of resources.

The Context has a list of values to free, but that's an unordered_map<long, m_value*> - which is released in

void ContextFree(ContextPtr ctx) {
  // ...
  for (auto it = ctx->vals.begin(); it != ctx->vals.end(); ++it) {
    auto value = it->second;
    value->ptr.Reset();
    delete value;
  }
  // ...
  delete ctx;
}

If this had been unordered_map<long, m_value> - that iteration shouldn't be necessary at all AFAICT - as that happens automatically in delete ctx

Btw, I think that there's some mechanism in Go - which is also used some places in the code to cleanup on GC - but not all places. So a lot of temporary Values are never freed until you delete the context. It's not a big problem for my intended use case as contexts are short-lived - but it's something I've wanted to look into.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a related note - in a different branch, I added helper functions to the struct, also to simplify commonly occurring patterns.

I'm hoping I'm not making a huge mistake - that a value could suddenly be used in a different isolate than the one that created it? If so, the iso should be an argument - and probably not present as a field in the struct.

struct m_value {
  long id;
   v8::Isolate* iso;
   m_ctx* ctx;
   v8::Global<v8::Value> ptr;
   v8::Local<v8::Value> ToLocal() { return this->ptr.Get(this->iso); }
 };

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, what's with the m_ prefix? Is it a C/C++ convention?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hoping I'm not making a huge mistake - that a value could suddenly be used in a different isolate than the one that created it? If so, the iso should be an argument - and probably not present as a field in the struct.

While Value itself doesn't seem tied to an Isolate, all concrete constructors (well New()) seem to take an Isolate. So I think we're good with that assumption.

m_ is used extensively in C++ standard libraries for member data, so it does feel weird to me too to have it on type names. And I can't find that it would be a libv8 thing either.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes - remember the m_ prefix from ATL I think (the MS lib to build COM objects). I adopted when I first started coding .NET 1.0 😆

m_value* val = new m_value;
val->id = 0;
val->iso = ctx->iso;
val->ctx = ctx;
val->ptr = Global<Value>(ctx->iso, value);
return tracked_value(ctx, val);
}

m_value* tracked_value(m_ctx* ctx, m_value* val) {
// (rogchap) we track values against a context so that when the context is
// closed (either manually or GC'd by Go) we can also release all the
Expand Down Expand Up @@ -119,3 +132,9 @@ RtnValue RunScript(ContextPtr ctx, const char* source, const char* origin) {
rtn.value = tracked_value(ctx, val);
return rtn;
}

m_ctx* m_ctx::FromV8Context(v8::Local<v8::Context> ctx) {
return (m_ctx*)ctx->GetEmbedderData(ContextDataIndex::WRAPPER_PTR)
.As<External>()
->Value();
}
24 changes: 24 additions & 0 deletions context.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,47 @@
#include "value.h"

namespace v8 {
class Value;
class Isolate;
class Context;
} // namespace v8

typedef v8::Isolate v8Isolate;
typedef struct m_unboundScript m_unboundScript;

// ContextDataIndex defines the indexes for "embedder data".
enum ContextDataIndex {
// We start at 1, as slot 0 has special meaning for the Chrome debugger

// Is an integer "handle" created in Go code, so given a specific V8 context,
// Go code can find its corresponding *Context value.
REF = 1,

// WRAPPER_PTR is the pointer being transferred between C++ and Go, a pointer
// to a C++ object responsible for maintenance tasks.
WRAPPER_PTR = 2,
};

// A wrapper on top of V8's Context object providing additional maintenance
// tasks for V8 go, e.g., releaseing unreleased Value objects associated with
// the context.
//
// Exactly one wrapper instance must exist for a V8 context. A pointer to the
// corresponding wrapper is stored as embedded data in the V8 context.
struct m_ctx {
v8::Isolate* iso;
std::unordered_map<long, m_value*> vals;
std::vector<m_unboundScript*> unboundScripts;
v8::Persistent<v8::Context> ptr;
long nextValId;

// Retrieves the wrapper context for a specific V8 context.
static m_ctx* FromV8Context(v8::Local<v8::Context> ctx);
};
typedef m_ctx* ContextPtr;

extern m_value* tracked_value(m_ctx* ctx, m_value* val);
extern m_value* track_value(m_ctx* ctx, v8::Local<v8::Value> val);

extern "C" {
#else
Expand Down
23 changes: 23 additions & 0 deletions isolate.cc
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#include "deps/include/v8-context.h"
#include "deps/include/v8-external.h"
#include "deps/include/v8-initialization.h"
#include "deps/include/v8-locker.h"
#include "deps/include/v8-platform.h"
#include "deps/include/v8-promise.h"

#include "_cgo_export.h"
#include "context.h"
#include "isolate.h"
#include "libplatform/libplatform.h"
Expand Down Expand Up @@ -74,6 +77,26 @@ int IsolateIsExecutionTerminating(IsolatePtr iso) {
return iso->IsExecutionTerminating();
}

void promiseRejectedCallback(v8::PromiseRejectMessage message) {
auto iso = message.GetPromise()->GetIsolate();

Local<Promise> prom = message.GetPromise();
auto handle = iso->GetData(1);
Local<Context> v8Ctx = prom->GetCreationContext(iso).ToLocalChecked();
int ctx_ref =
v8Ctx->GetEmbedderData(ContextDataIndex::REF).As<Integer>()->Value();
m_ctx* ctx = m_ctx::FromV8Context(v8Ctx);
Local<Value> val = message.GetValue();
goRejectedPromiseCallback(ctx_ref, handle, message.GetEvent(),
track_value(ctx, prom), track_value(ctx, val));
}

void IsolateSetPromiseRejectedCallback(IsolatePtr iso, void* handle) {
ISOLATE_SCOPE(iso)
iso->SetData(1, handle);
Comment thread
tommie marked this conversation as resolved.
iso->SetPromiseRejectCallback(promiseRejectedCallback);
}

IsolateHStatistics IsolationGetHeapStatistics(IsolatePtr iso) {
if (iso == nullptr) {
return IsolateHStatistics{0};
Expand Down
116 changes: 116 additions & 0 deletions isolate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,75 @@ package v8go
import "C"

import (
"runtime/cgo"
"strconv"
"sync"
"unsafe"
)

// PromiseRejectEvent represents the type of event passed to
// [RejectedPromiseCallback]. The values reflect the values of
// v8::PromiseRejectEvent.
//
// See also: https://v8.github.io/api/head/classv8_1_1PromiseRejectMessage.html
type PromiseRejectEvent uint8

const (
// PromiseRejectWithNoHandler is the event that represents an unhandled
// rejection.
PromiseRejectWithNoHandler PromiseRejectEvent = 0
// PromiseHandlerAddedAfterReject is sent when a rejection handler is added
// to a promise that has already rejected. E.g., the following code will
// result in a kPromiseRejectWithNoHandler event followed by an
// PromiseHandlerAddedAfterReject event.
//
// Promise.reject("dummy").catch(e => {})
//
// The promise has already rejected when catch is called.
PromiseHandlerAddedAfterReject PromiseRejectEvent = 1
// PromiseRejectAfterResolved is sent when a project is rejected after it
// has settled, e.g., the following will generate a
// PromiseRejectAfterResolved event.
//
// new Promise((resolve, reject) => {
// resolve()
// reject()
// })
//
// If the first resolve call is replaced with a reject, a
// kPromiseRejectWithNoHandler event is sent first, followed by the
// PromiseRejectAfterResolved event.
PromiseRejectAfterResolved PromiseRejectEvent = 2
// PromiseResolveAfterResolved is sent when a project is resolves after it
// has settled, e.g., the following will generate a
// PromiseResolveAfterResolved event.
//
// new Promise((resolve, reject) => {
// resolve() // or reject()
// resolve()
// })
//
// If the first resolve call is replaced with a reject, a
// kPromiseRejectWithNoHandler event is sent first, followed by the
// PromiseResolveAfterResolved event.
PromiseResolveAfterResolved PromiseRejectEvent = 3
)

func (u PromiseRejectEvent) String() string {
switch u {
case PromiseRejectWithNoHandler:
return "kPromiseRejectWithNoHandler"
case PromiseHandlerAddedAfterReject:
return "kPromiseHandlerAddedAfterReject"
case PromiseRejectAfterResolved:
return "kPromiseRejectAfterResolved"
case PromiseResolveAfterResolved:
return "kPromiseResolveAfterResolved"
default:
return strconv.Itoa(int(u))
}
}

// Isolate is a JavaScript VM instance with its own heap and
// garbage collector. Most applications will create one isolate
// with many V8 contexts for execution.
Expand All @@ -22,6 +87,7 @@ type Isolate struct {
cbMutex sync.RWMutex
cbSeq int
cbs map[int]FunctionCallbackWithError
handles []cgo.Handle

null *Value
undefined *Value
Expand Down Expand Up @@ -144,6 +210,9 @@ func (i *Isolate) Dispose() {
if i.ptr == nil {
return
}
for _, h := range i.handles {
h.Delete()
}
C.IsolateDispose(i.ptr)
i.ptr = nil
}
Expand Down Expand Up @@ -184,3 +253,50 @@ func (i *Isolate) getCallback(ref int) FunctionCallbackWithError {
defer i.cbMutex.RUnlock()
return i.cbs[ref]
}

//export goRejectedPromiseCallback
func goRejectedPromiseCallback(ctxref int, handle unsafe.Pointer, event PromiseRejectEvent, promise C.ValuePtr, value C.ValuePtr) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename handle to cb to link it to SetPromiseRejectedCallback(cb).

if p, err := (&Value{ptr: promise}).AsPromise(); err == nil {
msg := PromiseRejectMessage{
Context: getContext(ctxref),
Promise: p,
Event: event,
}
if value != nil {
msg.Value = &Value{ptr: value}
}
cb := (cgo.Handle)(handle).Value().(RejectedPromiseCallback)
cb(msg)
}
}

// PromiseRejectMessage is passed to a [RejectedPromiseCallback] that is
// installed using [Isolate.SetPromiseRejectedCallback]. The values reflect the
// values in V8::PromiseRejectMessage
//
// See also: https://v8.github.io/api/head/classv8_1_1PromiseRejectMessage.html
type PromiseRejectMessage struct {
// Context contains the execution context where the promise was rejected
Context *Context
Promise *Promise
Event PromiseRejectEvent
// Value contains the rejected value
Value *Value
}

// RejectedPromiseCallback is the type for a callback clients can supply to be
// notified of rejected promises.
type RejectedPromiseCallback = func(PromiseRejectMessage)

func (i *Isolate) addHandle(h cgo.Handle) cgo.Handle {
i.handles = append(i.handles, h)
return h
}

// SetPromiseRejectedCallback installs a callback to be called when a promise is
// rejected. This includes rejections that may occur after a script value has
// been evaluated and V8 is running microtasks.
func (i *Isolate) SetPromiseRejectedCallback(cb RejectedPromiseCallback) {
handle := unsafe.Pointer(uintptr(i.addHandle(cgo.NewHandle(cb))))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there was already a callback, this should deallocate the handle used by that callback. (which means I'm skeptical of the generic handles implementation.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - didn't think about this as I'd only set it up once.

Can you elaborate on the skeptical part?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a side note, The library has (at least) two Go error implementations. JSError and Exception.

*Exception represents a JavaScript Error object (at least "native errors" - don't know if class MyErr extends Error{} counts)

JSError is generated by v8go when a script invocation fails - but it doesn't include the thrown value.

In this particular case, the value is the *Exception value - so I can retrieve all the properties on the actual error object.

JSError does have some useful properties, e.g. for JavaScript code like throw "I'm a string but should have been an error object" - as you now have the stack trace - but the original error is missing, so if the error is returned back to JS - the original error value is lost. E.g., a function callback that itself calls back into JS - which fails with an error.

I might take a look at that eventually - but it's not a priority for my use cast ATM - just wanted to mention this.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on the skeptical part?

I think SetPromiseRejectedCallback should "own" its handle, which suggests the handle should be stored in a promiseRejectedCallback field, and not be mixed in with whatever else might end up in .handles. Right now, it's a philosophical question, since there's only one user of that field.

On a side note, The library has (at least) two Go error implementations. JSError and Exception.

I added Exception (rogchap#195) because there was no way to for functions to signal failure to Go code. It's one of the reasons for this fork. That created a discussion in rogchap#274, which stalled. Merging them would indeed be nice, and it sounds like I created a new type to avoid modifying too much of the code base without owner blessing.

@stroiman stroiman May 21, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SetPromiseRejectedCallback should "own" its handle

Ah, I think you're right, but maybe the promiseRejectedCallback shouldn't even have a handle. As I assume that only one callback can be set, it could just be a field on the Go Isolate? And then optionally a handle to the isolate?

type Isolate struct {
  selfHandle cgo.Handle
  promiseRejectedCallback RejectedPromiseCallback
}

//export goCallback
func goCallback(C.uintptr_t handle, /* ... reset */) {
  iso, ok := cgo.Handle(handle).Value().(*Isolate)
  iso.promiseRejectedCalback( /* ... */ );
}

func (i *Isolate) SetPromiseRejectedCallback(cb RejectedPromiseCallback) {
  if i.selfHandle == 0 { 
    i.selfHandle = cgo.NewHandle(i)
  }
  // This could potentially be moved _inside_ the if statement - as we just set the
  // same callback with the same handle. But could be dangerous if the handle is
  // used for different purposes.
  C.IsolateSetPromiseRejectedCallback(C.uintptrt(i.selfHandle), /* ... */ )
  ...
}

func (i *Isolate) Dispose() {
  if i.selfHandle != 0 { 
    i.selfHandle.Delete()
  }
}

@stroiman stroiman May 21, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Exception

Nice 💪 I'm so glad you did :) I also experimented with Goja to have a pure Go alternative - and it uses panics to return with errors, e.g., you need to panic(NewTypeError(...)) or something like that - but it also looses track of the error value. It feels very wrong.

The stalled discussion was titled, "Store error object in JSError for propagation". I would probably have suggested just to add a *Value field to JSError - as it's not only objects you can throw, e.g. throw 1 is valid, but poor JS. You might even make it an anonymous/embedded field, so all Value methods are promoted. I.e., it's just a value, but has additional information about error site.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side note on the side note. Exception values don't work with AsObject(), as it checks IsObject() - and that returns false for functions, exceptions, and module namespace objects.

In my ESM experimental branch, I have

func (v *Value) AsObject() (*Object, error) {
	if !v.IsObject() && !v.IsModuleNamespaceObject() && !v.IsFunction() && !v.IsNativeError() {
		return nil, errors.New("v8go: value is not an Object")
	}
	return &Object{v}, nil
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just checked the readme - I see you're also to thank for Symbol support 🙌 without which I wouldn't have proper iterables in my browser

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go vet is complaining that this is possible misuse of unsafe.Pointer, probably because of the uintptr cast.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - honestly, I am totally confused about the relation between C's uintptr_t and void* - maybe you know? (I have a strong suspicion your C/C++ knowledge far exceeds mine)

As far as I could tell, conversion between the two should be safe, and for passing cgo.Handle values in the ExternalValue branch, I passed the uintptr_t, and converts to/from void* in C++.

Any thoughts?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uintptr_t is an unsigned integer type as least as large as the platform's pointers. There's also intptr_t and ptrdiff_t for relative pointers. I.e. very similar in scope of size_t and ssize_t...

Seems you shouldn't use unsafe.Pointer with Handle, and instead do the conversion to pointer on the C-side: https://pkg.go.dev/runtime/cgo@go1.17#Handle

C.IsolateSetPromiseRejectedCallback(i.ptr, handle)
}
1 change: 1 addition & 0 deletions isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ extern IsolatePtr NewIsolate();
extern void IsolatePerformMicrotaskCheckpoint(IsolatePtr ptr);
extern void IsolateDispose(IsolatePtr ptr);
extern void IsolateTerminateExecution(IsolatePtr ptr);
extern void IsolateSetPromiseRejectedCallback(IsolatePtr iso, void* handle);
extern int IsolateIsExecutionTerminating(IsolatePtr ptr);
extern IsolateHStatistics IsolationGetHeapStatistics(IsolatePtr ptr);

Expand Down
73 changes: 72 additions & 1 deletion isolate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"math/rand"
"reflect"
"strings"
"testing"

Expand Down Expand Up @@ -130,7 +131,9 @@ func TestIsolateCompileUnboundScript_InvalidOptions(t *testing.T) {
CachedData: &v8.CompilerCachedData{Bytes: []byte("unused")},
Mode: v8.CompileModeEager,
}
panicErr := recoverPanic(func() { iso.CompileUnboundScript("console.log(1)", "script.js", opts) })
panicErr := recoverPanic(
func() { iso.CompileUnboundScript("console.log(1)", "script.js", opts) },
)
if panicErr == nil {
t.Error("expected panic")
}
Expand Down Expand Up @@ -255,6 +258,74 @@ func TestIsolateThrowException(t *testing.T) {
}
}

func TestIsolateSetPromiseRejectedCallback(t *testing.T) {
t.Parallel()
iso := v8.NewIsolate()
defer iso.Dispose()
ctx := v8.NewContext(iso)
defer ctx.Close()

var events []v8.PromiseRejectEvent
var lastValue *v8.Value

iso.SetPromiseRejectedCallback(func(msg v8.PromiseRejectMessage) {
events = append(events, msg.Event)
lastValue = msg.Value
})

_, err := ctx.RunScript("Promise.reject('value')", "")
fatalIf(t, err)

want := []v8.PromiseRejectEvent{v8.PromiseRejectWithNoHandler}
if !reflect.DeepEqual(events, want) {
t.Errorf("Unexpected events. Want: %v. Got: %v", want, events)
}
if lastValue == nil || lastValue.String() != "value" {
t.Errorf("Unexpected value. Want 'value', got: %v", lastValue)
}

events = nil
_, err = ctx.RunScript("Promise.reject('value').catch(err => { /* ignore */ })", "")
fatalIf(t, err)

want = []v8.PromiseRejectEvent{v8.PromiseRejectWithNoHandler, v8.PromiseHandlerAddedAfterReject}
if !reflect.DeepEqual(events, want) {
t.Errorf("Unexpected events. Want: %v. Got: %v", want, events)
}

testsWithMultipleResolutions := []struct {
op1 string // The first operation to run, resolve() or reject()
op2 string // The second operation to run, resolve() or reject()
want []v8.PromiseRejectEvent // The expected events generated
}{
{"resolve()", "resolve()", []v8.PromiseRejectEvent{v8.PromiseResolveAfterResolved}},
{"resolve()", "reject()", []v8.PromiseRejectEvent{v8.PromiseRejectAfterResolved}},
{
"reject()", "resolve()",
[]v8.PromiseRejectEvent{v8.PromiseRejectWithNoHandler, v8.PromiseResolveAfterResolved},
},
{
"reject()", "reject()",
[]v8.PromiseRejectEvent{v8.PromiseRejectWithNoHandler, v8.PromiseRejectAfterResolved},
},
}
for _, test := range testsWithMultipleResolutions {
name := fmt.Sprintf("Call %s after %s", test.op2, test.op1)
t.Run(name, func(t *testing.T) {
events = nil
ctx.RunScript(`
new Promise((resolve, reject) => {
`+test.op1 /* resolve() or reject() */ +`
`+test.op2 /* resolve() or reject() */ +`
})
`, "")
if !reflect.DeepEqual(test.want, events) {
t.Errorf("Unexpected events. Want: %v. Get: %v", test.want, events)
}
})
}
}

func BenchmarkIsolateInitialization(b *testing.B) {
b.ReportAllocs()
for n := 0; n < b.N; n++ {
Expand Down