Skip to content

Commit

Permalink
workaround tightening of c-go rules
Browse files Browse the repository at this point in the history
In go1.6, cgo rules regarding the passing of Go pointers to C libraries
changed to prevent unsafe interactions with the Go garbage collector.

We need to change so that we pass an indirect reference to the Go object
to the c-runtime.

This code has been tested to the extent that is required to address the
particular panic reported with #4

However, a more extensive regression test has not yet been performed.

Signed-off-by: Jon Seymour <[email protected]>
  • Loading branch information
Jon Seymour committed Mar 26, 2016
1 parent e77291c commit 62c8441
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 5 deletions.
6 changes: 5 additions & 1 deletion api.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import "C"
// ask an EventLoop to quit.

type api struct {
shareable
loop EventLoop
callback NotificationCallback
eventCallback EventCallback
Expand Down Expand Up @@ -69,7 +70,8 @@ func BuildAPI(configPath string, userPath string, overrides string) Configurator
//defer C.free(unsafe.Pointer(cUserPath))
//defer C.free(unsafe.Pointer(cOverrides)
C.startOptions(cConfigPath, cUserPath, cOverrides)
return &api{
var r *api
r = &api{
loop: defaultEventLoop,
callback: nil,
eventCallback: defaultEventCallback,
Expand All @@ -80,6 +82,8 @@ func BuildAPI(configPath string, userPath string, overrides string) Configurator
logger: &defaultLogger{},
networks: make(map[uint32]*network),
quitDeviceMonitor: make(chan int, 2)}
r.shareable.init(r)
return r
}

func (a *api) QuitSignal() chan int {
Expand Down
5 changes: 3 additions & 2 deletions api.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#ifndef API_H
#ifndef API_H
#define API_H
//
// api.h
Expand Down Expand Up @@ -39,14 +39,15 @@ extern "C" {
#else
#endif

#include "api/shareable.h"
typedef void API;

#include "api/manager.h"
#include "api/node.h"
#include "api/value.h"
#include "api/notification.h"
#include "api/options.h"


#ifdef __cplusplus
#include "_cgo_export.h"
}
Expand Down
5 changes: 5 additions & 0 deletions api/shareable.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
typedef struct shareable {
int sharedIndex;
} shareable;

shareable * newShareable(int i);
13 changes: 13 additions & 0 deletions api_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package openzwave

import "testing"

func Test_RoundtripMarshaling(t *testing.T) {
a := &api{}
a.init(a)
c := a.C()
aa := unmarshal(c).Go().(*api)
if a != aa {
t.Fatalf("failed to round trip")
}
}
6 changes: 4 additions & 2 deletions run.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (a *api) Run() int {
//

go func() {
cSelf := unsafe.Pointer(a) // a reference to a
cSelf := a.C()

C.startManager(cSelf) // start the manager
defer C.stopManager(cSelf)
Expand Down Expand Up @@ -214,12 +214,14 @@ func (a *api) Shutdown(exit int) {
default:
}

a.shareable.destroy()

}

//export onNotificationWrapper
func onNotificationWrapper(cNotification *C.Notification, context unsafe.Pointer) {
// marshal from C to Go
a := (*api)(context)
a := unmarshal(context).Go().(*api)
goNotification := newGoNotification(cNotification)
if a.callback != nil {
a.callback(a, goNotification)
Expand Down
7 changes: 7 additions & 0 deletions shareable.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include "api.h"

shareable * newShareable(int i) {
shareable * r = (shareable *) malloc(sizeof(shareable));
r->sharedIndex = i;
return r;
}
83 changes: 83 additions & 0 deletions shareable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package openzwave

// #cgo LDFLAGS: -lopenzwave -Lgo/src/github.com/ninjasphere/go-openzwave/openzwave
// #cgo CPPFLAGS: -Iopenzwave/cpp/src/platform -Iopenzwave/cpp/src -Iopenzwave/cpp/src/value_classes
// #include "api.h"
import "C"

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

// This module fixes an issue revealed when Go 1.6 tightened up the rules
// about sharing of Go pointers with C code. https://github.com/golang/go/issues/12416
//
// Now we register a reference with a map and get a pointer to a structure in the C heap on return.
// We use the integer recorded in this structure to map back into the Go world.
//
// That way there are O(1) marshaling operations, and Go pointer is never shared with or
// dereferenced by the C world
//
var shared = map[int]Shareable{}
var sharedCount = 0
var mu sync.RWMutex

type Shareable interface {
C() unsafe.Pointer
Go() interface{}
}

type shareable struct {
cref *C.shareable
goObject interface{}
}

func (s *shareable) init(goObject interface{}) {
mu.Lock()
defer mu.Unlock()

sharedCount++
s.cref = C.newShareable(C.int(sharedCount))
shared[int(s.cref.sharedIndex)] = s
s.goObject = goObject
}

func (s *shareable) destroy() {
if s.cref != nil {
mu.Lock()
defer mu.Unlock()
delete(shared, int(s.cref.sharedIndex))
C.free(s.C())
s.cref = nil
}
}

func (s *shareable) C() unsafe.Pointer {
return unsafe.Pointer(s.cref)
}

func (s *shareable) Go() interface{} {
if s == nil {
return nil
} else {
return s.goObject
}
}

func unmarshal(c unsafe.Pointer) Shareable {
mu.RLock()
defer mu.RUnlock()

i := (*C.shareable)(c).sharedIndex
if i == 0 {
return nil
} else {
if s, ok := shared[int(i)]; !ok {
panic(fmt.Errorf("failure to unmarshal index %d", int(i)))
} else {
return s
}
}
}

0 comments on commit 62c8441

Please sign in to comment.