NewContext may panic due to cgo pointer passing rules (infrequent)

[dmitshur]

This is a panic I've observed during a call to the oto.NewContext function (using latest version, commit 98bd5f4) on macOS:

$ go version
go version go1.15.2 darwin/amd64
$ go run .
source sample rate: 44100
playing back at: 44100
panic: runtime error: cgo argument has Go pointer to Go pointer

goroutine 1 [running]:
github.com/hajimehoshi/oto.setNotificationHandler.func1(0xc0000ee000)
	github.com/hajimehoshi/oto/driver_darwin.go:320 +0x47
github.com/hajimehoshi/oto.setNotificationHandler(0xc0000ee000)
	github.com/hajimehoshi/oto/driver_darwin.go:320 +0x2b
github.com/hajimehoshi/oto.setDriver(0xc0000ee000)
	github.com/hajimehoshi/oto/driver_darwin.go:77 +0xb8
github.com/hajimehoshi/oto.newDriver(0xac44, 0x2, 0x2, 0x10000, 0xc00004fe40, 0x40b1997, 0xc00007c000, 0xc00001a120)
	github.com/hajimehoshi/oto/driver_darwin.go:141 +0x31e
github.com/hajimehoshi/oto.NewContext(0xac44, 0x2, 0x2, 0x10000, 0x0, 0x0, 0x0)
	github.com/hajimehoshi/oto/context.go:69 +0xd6
main.playMP3(0x4101e17, 0x2c, 0x0, 0x0)
	github.com/shurcooL/play/239/main.go:40 +0x239
main.main()
	github.com/shurcooL/play/239/main.go:19 +0x36
exit status 2

It has happened once out of many repeated runs, so it's quite rare.

[hajimehoshi]

Thanks, I'll take a look!

[hajimehoshi]

A Go pointer is not passed to oto_setNotificationHandler, so this seems odd. (Is this a false positive?)

Now the argument at oto_setNotificationHandler is not used, I'll remove this.

[hajimehoshi]

I'm not sure this was fixed, but I removed the argument that the stacktrace says.

@dmitshur Could you try the latest commit?

[dmitshur]

I've tried running the program around 10 times before and after commit 90150a1, and this hasn't happened. Given how rare it was, it's hard to say.

Given the argument is completely gone now, I think this must be fixed, so I suggest closing this issue. It can be re-opened if anyone runs into this again.

[dmitshur]

A Go pointer is not passed to oto_setNotificationHandler, so this seems odd. (Is this a false positive?)

I'm not sure, but here's one guess. One of the parameters to the C.AudioQueueNewOutput function is &desc, which is a pointer to memory allocated by Go. I don't know what C.AudioQueueNewOutput does, but perhaps it modifies &audioQueue such that it keeps &desc. But then I can't explain why it's not more reproducible.

I tried GODEBUG=cgocheck=2 a few times, but it didn't report anything more than the default.

Finally, it could be related to the "Note: the current implementation has a bug. While Go code is permitted to write nil or a C pointer (but not a Go pointer) to C memory, the current implementation may sometimes cause a runtime error if the contents of the C memory appear to be a Go pointer. [...]" paragraph in https://golang.org/cmd/cgo/#hdr-Passing_pointers.

[hajimehoshi]

I see. Let's close this, and revisit this when necessary.

Your explanation seems to make sense. The odd thing is, as you have already pointed out, this is not deterministic. And this might also be a current known issue in the Go's pointer detection, which also makes sense. As long as this is not deterministic, it is pretty hard to find the culprit.

Thank you very much!