From 2fa955389e69c8cbc8a7b9f4ab6c05c546d7b9c0 Mon Sep 17 00:00:00 2001
From: Adam S Levy <adam@aslevy.com>
Date: Sat, 16 Nov 2019 10:47:21 -0900
Subject: [PATCH] fix(InstanceContext): Don't use uintptr for Data

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
---
 wasmer/import.go   | 11 +++++-----
 wasmer/instance.go | 55 +++++++++++++++++++++++++++++++++-------------
 2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/wasmer/import.go b/wasmer/import.go
index 390038ce..4c9d57ae 100644
--- a/wasmer/import.go
+++ b/wasmer/import.go
@@ -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]
 }
diff --git a/wasmer/instance.go b/wasmer/instance.go
index 3d0d52e6..1330e878 100644
--- a/wasmer/instance.go
+++ b/wasmer/instance.go
@@ -2,6 +2,8 @@ package wasmer
 
 import (
 	"fmt"
+	"runtime"
+	"sync"
 	"unsafe"
 )
 
@@ -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.
@@ -406,6 +404,17 @@ 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,
@@ -413,16 +422,32 @@ func (instance *Instance) HasMemory() bool {
 // 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`.