Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go 1.6 cgo new pointer rules #18

Open
fabiofalci opened this issue Mar 7, 2016 · 4 comments
Open

Go 1.6 cgo new pointer rules #18

fabiofalci opened this issue Mar 7, 2016 · 4 comments

Comments

@fabiofalci
Copy link
Contributor

Go 1.6 cgo has new rules about passing pointers.
https://github.com/golang/proposal/blob/master/design/12416-cgo-pointers.md

When compiling go-libspotify with 1.6 I'm getting this error:

panic: runtime error: cgo argument has Go pointer to Go pointer

Triggered by this line https://github.com/op/go-libspotify/blob/master/spotify/libspotify.go#L238

err := spError(C.sp_session_create(&session.config, &session.sp_session))

Just to register as I didn't have too much time to looking into this yet.

@NeonMonk
Copy link

Did you get anywhere with this? Or do we have to downgrade Go to get libspotify working?

@fabiofalci
Copy link
Contributor Author

Yes I had to downgrade to 1.5 in https://github.com/fabiofalci/sconsify
That or to use GODEBUG=cgocheck=0 to ignore the issue.

@CyrusRoshan
Copy link

Kind of sketchy, but I used this to get around it by calling the same program with the same env/arg/flags plus the GODEBUG flag.

@fabiofalci
Copy link
Contributor Author

@op I've been avoiding this for a long time, but now I had to try to fix as the current version of https://github.com/fabiofalci/sconsify is freezing on macOS Sierra.

After some tests, I realised that it doesn't freeze if compiled with go 1.8 - using GODEBUG=cgocheck=0 to ignore the new rules.

I must say I'm not fluent in C, not even close, but I had to come up with something:

fabiofalci@fbe297f

We got 2 places sending these pointers to C: session.config.callbacks and session.config.userdata.

The first one I moved the creation to C and then returning to GO to pass to C.sp_session_create.

The second one I removed altogether, not passing anything.

Both solutions are based on the assumption that it'll never try to support multi sessions. Decided to go for this solution because of this comment on sp_session_create's libspotify: Currently it is not supported to have multiple active sessions, and it's recommended to only call this once per process. And as libspotify is deprecated that's unlikely to change.

So the session is kept as a variable and used as needed instead of using the one returning from callbacks. And the callbacks struct is now being created on C instead of GO.

Please let me know what you think. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants