New Go<->cgo pointer rules in 1.6 don't like gocairo

[dgryski]

While running https://github.com/dgryski/carbonapi with Go 1.6rc2, I got

http: panic serving 127.0.0.1:44456: runtime error: cgo argument has Go pointer to Go pointer

This error didn't happen with 1.5.

[dgryski]

More panic output:

goroutine 20 [running]:
net/http.(*conn).serve.func1(0xc8200f2980)
    /home/dgryski/work/src/cvs/go/src/net/http/server.go:1389 +0xc1
github.com/martine/gocairo/cairo._cgoCheckPointer0(0x89fbe0, 0xc8200de8a0, 0xc8204b4f90, 0x1, 0x1, 0x876840)
    ??:0 +0x4d
github.com/martine/gocairo/cairo.(*Surface).WriteToPNG(0xc820178058, 0x7faf32cfc160, 0xc8200f0cc0, 0x0, 0x0)
    /home/dgryski/work/src/cvs/gocode/src/github.com/martine/gocairo/cairo/cairo.go:63 +0x202
...

[evmar]

Thanks for the report!

It's because I pass one of these through C:

type writeClosure struct {
    w   io.Writer
    err error
}

and I guess that's no longer allowed.

I'm not exactly sure how they expect me to wrap an API like this
http://cairographics.org/manual/cairo-PNG-Support.html#cairo-surface-write-to-png-stream
in Go 1.6. Apparently I can't use any Go pointers in callbacks? But functions like this are pretty common in C code, so there must be some way.

[dgryski]

Probably need to ask Ian on golang-nuts.. I'm sure lots of people will be hit by this.

[evmar]

I got a comment from Ian himself! The design doc suggests this pattern:

One safe approach is: Go code that wants to preserve funcs/pointers stores them into a map indexed by an int. Go code calls the C code, passing the int, which the C code may store freely. When the C code wants to call into Go, it passes the int to a Go function that looks in the map and makes the call. An explicit call is required to release the value from the map if it is no longer needed, but that was already true before.

I will write a patch this weekend hopefully.

[stephenwithav]

Were you able to find a solution?

[evmar]

I talked to Ian separately. The fix unfortunately is to use a map of pending writes. I haven't had time to work on this but I hope to get to it soon.

See
https://github.com/martine/gocairo/blob/master/cairo/util.go#L39

E.g.

var closures = map[int]*writeClosure{}
var closuresLock sync.Mutex

// Then when calling into the C API from our wrapper, something like:
closuresLock.Lock()
closures[x] = &writeClosure{...}
closuresLock.Unlock()
C.cairo_whatever(..., x)
closuresLock.Lock()
delete(closures, x)
closuresLock.Unlock()
...

and similarly on the other side we look up the closure with a key.

This is what Ian suggested anyway.  Pretty gross.

[stephenwithav]

Is that conversation public? I don't understand how a mutex would change things. (The mutex is needed for concurrent map access, but the writeClosure would still contain a Go pointer.)

[dgryski]

Yes, but instead of passing the writeClosure to C, you pass the key (an integer) to the entry in the map.

[stephenwithav]

How would you reference the map from C?

[evmar]

Please let me know if 6a65339 (on master) fixes the problem. I wrote it blind (I don't have Go 1.6 locally) but I think it should work.

[evmar]

Oops, and to answer the earlier question, no the conversation wasn't public. But as I pasted above the design doc from when they made this change had a sentence or two about doing this.

[dgryski]

Yup; works here. Thanks!

[evmar]

You're the first user of this library I've heard from, so please feel free to make feature requests etc. if you have any!

[stephenwithav]

Everything works here, too. Thank you, @martine!

[dgryski]

@martine The only feature I'm missing is svg output. I'll file a bug for that.