diff --git a/isolate.go b/isolate.go index bac63e1..c0cdd88 100644 --- a/isolate.go +++ b/isolate.go @@ -342,14 +342,18 @@ func (i *Isolate) Dispose() { unregisterIsolate(i) // Serialise dispose against snapshotDeserMu: V8 14.x corrupts its // shared-heap teardown state if a parallel goroutine is constructing - // a new isolate at the same moment we are tearing one down. The - // critical section is short (microseconds) and idiomatic v8::Locker - // rules continue to apply for everyday API calls. + // a new isolate at the same moment we are tearing one down. + // CreateCodeCache takes an RLock on the same mutex so it cannot + // overlap with Dispose — see unbound_script.go. + // + // i.ptr is set to nil inside the lock so that any RLock holder + // that re-checks after acquiring the lock sees the nil and bails + // instead of passing a dangling pointer to CGo. snapshotDeserMu.Lock() C.IsolateDisposeSnapshot(i.ptr) C.IsolateDispose(i.ptr) - snapshotDeserMu.Unlock() i.ptr = nil + snapshotDeserMu.Unlock() if i.snapshotData != nil { C.free(i.snapshotData) i.snapshotData = nil diff --git a/snapshot.go b/snapshot.go index 83739d9..079a173 100644 --- a/snapshot.go +++ b/snapshot.go @@ -86,14 +86,23 @@ var ErrSnapshotCreatorConsumed = errors.New( // lets the embedder use the wrapper freely from any goroutine. var snapshotCreatorMu sync.Mutex -// snapshotDeserMu serialises every Isolate::New call. The V8 binaries we -// ship via tommie/v8go/deps assert and abort when two isolates are being +// snapshotDeserMu serialises Isolate::New and Isolate::Dispose against +// each other AND against CreateCodeCache. The V8 binaries we ship via +// tommie/v8go/deps assert and abort when two isolates are being // constructed in parallel because the shared-heap initialiser (string // table forwarding, read-only heap shrink, etc) is not thread-safe in -// the absence of v8::Locker. Holding this mutex around every isolate -// construction keeps the wrapper safe to call from any goroutine, with -// no cost outside the construction critical section. -var snapshotDeserMu sync.Mutex +// the absence of v8::Locker. +// +// RWMutex semantics: +// - NewIsolate / Dispose take Lock (exclusive writer) because they +// mutate V8 process-global shared-heap state. +// - CreateCodeCache takes RLock (shared reader) because it reads +// shared-heap objects (ReadOnlySpace roots, StringTable) that +// Dispose can tear down concurrently. +// +// Multiple CreateCodeCache calls on different isolates remain parallel; +// they only block while an isolate is being created or disposed. +var snapshotDeserMu sync.RWMutex // NewSnapshotCreator returns a new SnapshotCreator wired to the frozen // process-wide external_references registry. The very first call to this diff --git a/unbound_script.cc b/unbound_script.cc index 7a30f4a..d3265b5 100644 --- a/unbound_script.cc +++ b/unbound_script.cc @@ -12,6 +12,7 @@ using namespace v8; ScriptCompilerCachedData* UnboundScriptCreateCodeCache( Isolate* iso, UnboundScriptPtr us_ptr) { + if (!iso || !us_ptr) return nullptr; ISOLATE_SCOPE(iso); Local unbound_script = us_ptr->ptr.Get(iso); diff --git a/unbound_script.go b/unbound_script.go index dbe5ff9..da36f12 100644 --- a/unbound_script.go +++ b/unbound_script.go @@ -22,9 +22,32 @@ func (u *UnboundScript) Run(ctx *Context) (*Value, error) { return valueResult(ctx, rtn) } -// Create a code cache from the unbound script. +// CreateCodeCache serialises the compiled bytecode so it can be fed to +// a future CompileUnboundScript call (on any isolate) to skip parsing. +// +// An RLock on snapshotDeserMu is held for the duration of the CGo call +// so that Isolate.Dispose (which takes a write Lock) cannot tear down +// V8 shared-heap state while CreateCodeCache is reading it. Multiple +// concurrent CreateCodeCache calls on different isolates remain parallel. +// +// Returns nil without calling into V8 if the isolate has already been +// disposed (ptr set to nil inside Dispose's write lock). func (u *UnboundScript) CreateCodeCache() *CompilerCachedData { + if u == nil || u.iso == nil { + return nil + } + + snapshotDeserMu.RLock() + defer snapshotDeserMu.RUnlock() + + if u.iso.ptr == nil { + return nil + } + rtn := C.UnboundScriptCreateCodeCache(u.iso.ptr, u.ptr) + if rtn == nil { + return nil + } cachedData := &CompilerCachedData{ Bytes: []byte(C.GoBytes(unsafe.Pointer(rtn.data), rtn.length)),