Skip to content
This repository has been archived by the owner on Aug 31, 2020. It is now read-only.

xfer_callback panic #50

Closed
codido opened this issue Jun 9, 2017 · 5 comments
Closed

xfer_callback panic #50

codido opened this issue Jun 9, 2017 · 5 comments

Comments

@codido
Copy link

codido commented Jun 9, 2017

I recently ran into a few cases in which the channel in xfer_callback was either nil or already closed, causing a panic.

Looking at the code, I'm not sure the usage of the user_data member (which holds the done channel) is valid. According to CGO's documentation, C may not keep a copy of a Go pointer after the call returns, and it looks like the case here.

One workaround for this is to hold a mapping from IDs to usbTransfer pointers, and set user_data to the ID rather than to the done channel. xfer_callback can then retrieve the relevant usbTransfer pointer and close its done channel.

@zagrodzki
Copy link
Collaborator

I think you are correct and I've experienced it before as well. It seems to be dependent on the Go and GCC version as far as I can tell. I'll add a fix later, in the meantime, you can try out the new release of the library that solved this problem (in exactly the way you described).

https://github.com/google/gousb

It's not been formally announced yet as the stable replacement, but I think we're getting there.

@codido
Copy link
Author

codido commented Jun 9, 2017

Ah, good to know, thanks!

@codido
Copy link
Author

codido commented Aug 28, 2017

btw, I believe there's another similar issue in the code, which google/gousb also experiences:
libusb.alloc stores the buffer (Go pointer) into the allocated xfer structure, which is later on used by C.

@zagrodzki
Copy link
Collaborator

I think you're right - the xfer struct is C memory (allocated by libusb), &buf[0] is a Go pointer, and "Go code may not store a Go pointer in C memory". It seems like the buffer should be initialized from the C side, since then the ownership of the buffer would be controlled by the C code and the problem would go away. I'll try to fix it sometime soon.

@zagrodzki
Copy link
Collaborator

google/gousb fixed in google/gousb#11

Since kylelemons/gousb was now officially marked as deprecated, I'll close this issue, since I won't be doing any more bugfixes in this code.

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

No branches or pull requests

2 participants