Question about thread safety

[jmattheis]

The documentation of v8go.NewIsolate() says Isolates aren't thread safe

v8go/isolate.go

Lines 47 to 49 in d8d94c2

	// NewIsolate creates a new V8 isolate. Only one thread may access
	// a given isolate at a time, but different threads may access
	// different isolates simultaneously.

Promise#then says Context#PerformMicrotaskCheckpoint must be executed until the promise is resolved.

v8go/promise.go

Lines 93 to 95 in d8d94c2

	// V8 only invokes the callback when processing "microtasks".
	// The default MicrotaskPolicy processes them when the call depth decreases to 0.
	// Call (*Context).PerformMicrotaskCheckpoint to trigger it manually.

When one goroutine executes Context#PerformMicrotaskCheckpoint and another one uses the isolate for v8.Null() or v8.NewValue() to create the resolve promise value, does this create thread safety problems, because the isolate is used in two different goroutines? How can I find out, if something can be used in different goroutines like v8.NewValue?

Here simple program to reproduce, it is similar to the example in the PR that added support for promises #76 (comment)

Simple Program

func main() {
	iso := v8.NewIsolate()
	sleep := v8.NewFunctionTemplate(iso, func(info *v8.FunctionCallbackInfo) *v8.Value {
		ctx := info.Context()
		p, err := v8.NewPromiseResolver(ctx)
		if err != nil {
			panic(err)
		}
		go func() {
			time.Sleep(1 * time.Second)
			p.Resolve(v8.Null(iso))
		}()
		return p.Value
	})
	global := v8.NewObjectTemplate(iso)
	global.Set("sleep", sleep)
	ctx := v8.NewContext(iso, global)
	_, err := ctx.RunScript(`const test = async () => { await sleep(); };`, "sleep.js") // executes a script on the global context
	if err != nil {
		panic(err)
	}
	val, err := ctx.RunScript("test()", "exec.js")
	if err != nil {
		panic(err)
	}
	p, _ := val.AsPromise()

	p.Then(func(info *v8.FunctionCallbackInfo) *v8.Value {
		fmt.Println("promise done")
		return v8.Null(iso)
	})
	fmt.Println("waiting")
	for p.State() == v8.Pending {
		ctx.PerformMicrotaskCheckpoint()
	}

	fmt.Println("done done")
}

[dylanahsmith]

It seems like the documentation has already answered your question, so your example isn't supported.

Instead of resolving the promise from another goroutine, send the result over a channel to resolver it from the isolate's goroutine.

The one exception is Isolate.TerminateExecution which is intended to be used from another goroutine, since it is needed in order to interrupt JS execution (e.g. to enforce a deadline). It looks like that is missing from the documentation at the moment.

[jmattheis]

thanks.

[jmattheis]

@dylanahsmith Is the example at https://pkg.go.dev/rogchap.com/v8go#example-FunctionTemplate-Fetch faulty, because it uses the iso (for creating the string(body) outside of the goroutine that has created the isolate?

[dylanahsmith]

Good point.

For context, v8go does internally use v8::Locker on the isolate, which provides some type safety. However, the primary reason for its use is to handle the fact that goroutines switch which thread they are run on and taking that lock will get v8 to update a thread-local stack limit so that V8 can handle the thread switch.

I think that example of using (*PromiseResolver).Resolve probably would work given the current use of v8::Locker, but I'm not sure if that would change in the future. However, v8go also has wrapper Go structs around V8 data which aren't type safe. That means relying on that could result in breaking changes when upgrading v8go, such as from state being moved between C++ structs and Go structs (e.g. as part of addressing #105).

It seems like that example should be modified to not rely the method being type safety to reflect what is actually supported. I'll reopen this since this does reflect an issue with the documentation being inconsistent.

There is another issue with more discussion about thread-safety: #129

[rutsky]

It seems like the documentation has already answered your question, so your example isn't supported.

Instead of resolving the promise from another goroutine, send the result over a channel to resolver it from the isolate's goroutine.

So you can't call PromiseResolver.Resolve() from a separate goroutine that could potentially ran in parallel with other javascript code for the same Isolate in V8?

And examples like

• in the first comment in this thread,

• v8go/function_template_fetch_test.go

  Lines 33 to 37 in cee5f84

  	go func() {
  	res, _ := http.Get(url)
  	body, _ := ioutil.ReadAll(res.Body)
  	val, _ := v8.NewValue(iso, string(body))
  	resolver.Resolve(val)

• fetch() implementation in v8go-polyfills by @kuoruan : https://github.com/kuoruan/v8go-polyfills/blob/c74c5b408ebdd76b65a26c693cf6cd612b2dace7/fetch/fetcher.go#L96-L101

are buggy and likely cause eventual crashes?