Skip to content

Commit

Permalink
fix(InstanceContext): Don't use uintptr for Data
Browse files Browse the repository at this point in the history
Previously an uintptr to user data was passed across the CGo FFI which
would become invalid if any stack frame occurred. This commit avoids
this by never passing any pointers across the CGo FFI. A registry map is
used instead to keep Go data on the Go side, and only an int index into
the map is passed across to the C side.

fix #93
re #44 #85 #83
  • Loading branch information
AdamSLevy committed Dec 3, 2019
1 parent bf837c2 commit 2fa9553
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 20 deletions.
11 changes: 6 additions & 5 deletions wasmer/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,11 @@ func (instanceContext *InstanceContext) Memory() *Memory {
return &instanceContext.memory
}

// Data returns the instance context data as an `interface{}`. It's
// up to the user to cast it appropriately.
// Data returns the instance context data as an `interface{}`. It's up to the
// user to assert the proper type.
func (instanceContext *InstanceContext) Data() interface{} {
dataPtr := *(*unsafe.Pointer)(cWasmerInstanceContextDataGet(instanceContext.context))

return *(*interface{})(dataPtr)
ctxDataIdx := *(*int)(cWasmerInstanceContextDataGet(instanceContext.context))
ctxDataMtx.RLock()
defer ctxDataMtx.RUnlock()
return ctxData[ctxDataIdx]
}
55 changes: 40 additions & 15 deletions wasmer/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package wasmer

import (
"fmt"
"runtime"
"sync"
"unsafe"
)

Expand Down Expand Up @@ -72,11 +74,7 @@ type Instance struct {
// The exported memory of a WebAssembly instance.
Memory *Memory

// `contextDataC` and `contextDataGo` ensure thata`
// InstanceContext` data is not garbage-collected // for the
// lifetime of the `Instance`.
contextDataC *uintptr
contextDataGo interface{}
ctxDataIdx *int
}

// NewInstance constructs a new `Instance` with no imported functions.
Expand Down Expand Up @@ -406,23 +404,50 @@ func (instance *Instance) HasMemory() bool {
return nil != instance.Memory
}

var (
// In order to avoid passing illegal Go pointers across the CGo FFI,
// store Instance Context Data in ctxData and simply pass the index
// through the FFI instead.
//
// See Instance.SetContextData and InstanceContext.Data.
ctxData = make(map[int]interface{})
nextCtxDataIdx int
ctxDataMtx sync.RWMutex
)

// SetContextData assigns a data that can be used by all imported functions.
// Each imported function receives as its first argument an instance context
// (see `InstanceContext`). An instance context can hold any kind of data,
// including data that contain Go references such as slices, maps, or structs
// with reference types or pointers. It is important to understand that data is
// global to the instance, and thus is shared by all imported functions.
func (instance *Instance) SetContextData(data interface{}) {
// Cache the data with the `new(uintptr)` to prevent it to be
// garbage collected for the lifetime of the instance.
instance.contextDataGo = data
instance.contextDataC = new(uintptr)
*instance.contextDataC = uintptr(unsafe.Pointer(&instance.contextDataGo))

cWasmerInstanceContextDataSet(
instance.instance,
unsafe.Pointer(instance.contextDataC),
)
ctxDataMtx.Lock()
if instance.ctxDataIdx == nil {
instance.ctxDataIdx = new(int)
*instance.ctxDataIdx = nextCtxDataIdx
nextCtxDataIdx++

// When instance is GC'd, clean up its ctxData.
// Set the finalizer on the unexported instance.ctxDataIdx,
// instead of directly on the instance, to allow users of this
// package to set their own finalizer on the Instance for other
// reasons.
runtime.SetFinalizer(instance.ctxDataIdx, func(idx *int) {
// Launch a goroutine to avoid blocking other
// finalizers while waiting for the mutex lock.
go func() {
ctxDataMtx.Lock()
delete(ctxData, *idx)
ctxDataMtx.Unlock()
}()
})
}
ctxData[*instance.ctxDataIdx] = data
ctxDataMtx.Unlock()

cWasmerInstanceContextDataSet(instance.instance,
unsafe.Pointer(instance.ctxDataIdx))
}

// Close closes/frees an `Instance`.
Expand Down

0 comments on commit 2fa9553

Please sign in to comment.