{"action":"created","issue":{"url":"https://api.github.com/repos/trezor/trezord-go/issues/108","repository_url":"https://api.github.com/repos/trezor/trezord-go","labels_url":"https://api.github.com/repos/trezor/trezord-go/issues/108/labels{/name}","comments_url":"https://api.github.com/repos/trezor/trezord-go/issues/108/comments","events_url":"https://api.github.com/repos/trezor/trezord-go/issues/108/events","html_url":"https://github.com/trezor/trezord-go/pull/108","id":365492831,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE5NDAwMDk4","number":108,"title":"Cleanup of OS-specific (freebsd) code","user":{"login":"karel-3d","id":104945,"node_id":"MDQ6VXNlcjEwNDk0NQ==","avatar_url":"https://avatars2.githubusercontent.com/u/104945?v=4","gravatar_id":"","url":"https://api.github.com/users/karel-3d","html_url":"https://github.com/karel-3d","followers_url":"https://api.github.com/users/karel-3d/followers","following_url":"https://api.github.com/users/karel-3d/following{/other_user}","gists_url":"https://api.github.com/users/karel-3d/gists{/gist_id}","starred_url":"https://api.github.com/users/karel-3d/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/karel-3d/subscriptions","organizations_url":"https://api.github.com/users/karel-3d/orgs","repos_url":"https://api.github.com/users/karel-3d/repos","events_url":"https://api.github.com/users/karel-3d/events{/privacy}","received_events_url":"https://api.github.com/users/karel-3d/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-10-01T14:38:20Z","updated_at":"2018-10-02T16:19:22Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/trezor/trezord-go/pulls/108","html_url":"https://github.com/trezor/trezord-go/pull/108","diff_url":"https://github.com/trezor/trezord-go/pull/108.diff","patch_url":"https://github.com/trezor/trezord-go/pull/108.patch"},"body":"I have done a small cleanup of OS-specific code.\r\n\r\ntl;dr:\r\n\r\n* On freebsd, do not allow session stealing, as it's too complex to do with system libusb.\r\n* On freebsd and linux, use libusb even for the current HID trezor.\r\n* rename webusb.go/WebUSB to libusb.go/LibUSB\r\n\r\n---\r\n\r\nThe initial problem I was trying to solve is the 5-second timeouts still create issues on FreeBSD. \r\n\r\n---\r\n\r\nTo explain why do we need all this complicated machinery. For better user experience, we allow for \"multitasking\" - that is, applications can \"steal\" sessions from each other when device is in session, and that should cancel the communication. The result after \"session stealing\" should be a device that is capable of communication; after a session is stolen and after the stealing application calls an \"initialize\" message, the device should be in a fresh state and reply message with features.\r\n\r\nWhy is this done? User can have many windows with many applications open, but want to communicate with just one website on the foreground. Trezor starts to automatically communicate with some background tab - but user can steal the session from the background tab in the foreground tab (that has the app that he wants to use) and starts using trezor (and the background tab action fails, but user doesn't care about that). No background tab can \"block\" using trezor.\r\n\r\nHow to simulate this (there is a bug in web wallet now that does make it a little complicated :(( ):\r\n* open trezor web wallet in one tab, clear all saved devices\r\n* connect a device, do all the initialization dance (pin, passphrase, load accounts)\r\n* disconnect device, click on \"save\", device is now saved\r\n* connect the device again\r\n* do some action that starts a session but doesn't immedialy end it - for example, displaying address and keep waiting\r\n* open in another tab another web wallet\r\n* now it should say \"used in another window\", but clicking on the icon next to it should end the previous session and start with a clean slate (sending of the \"initialize\" message here is, however, up to the website, bridge doesn't do that)\r\n\r\n----\r\n\r\nThis is rather complicated in reality, since there are interlocking issues here.\r\n\r\n* for reason I cannot remember - maybe this issue https://github.com/libopencm3/libopencm3/issues/668 maybe some other - we had a problem on some system with enumerating the devices while there was a transfer going on. So I have added a mutex that does not allow the enumeration while a transfer is happening (callInProgress and callMutex in core); if an enumeration request comes during any transfer, we just reply with the last enumeration result; however, that also means that if communication hangs on forever, enumeration is stuck\r\n* similarly, we want closing to be atomic - so if we close the device, we want to wait until closing to reply in the enumeration that the device is free to use\r\n* however, because of some trezor t HW issues, we still return some data on the bus even when the transfer is cancelled, IF we cancel it mid-transfer (what I wrote about session stealing, it can happen mid read) - so we need to do some \"finishing read queue\" (fininshReadQueue) - reading data with very short timeouts \r\n* and libusb sometimes hangs if you close a device in the middle of a transfer, you dont cancel the transfer, and then you try to read or write on the device again - so the close() then hangs because finishReadQueue() hangs and that makes enumeration stuck\r\n\r\n---\r\n\r\n.....anyway. The whole point of this is - before, we solved all those issues with interrupt transfers with timeouts. Since that sort of worked - the read/write never hangs, unless you do two of them simultaneously or unless you close the device below it, so we added transferMutex for that.\r\n\r\nBut now we found out that the timeout transfers cause problems, so instead, we used transfers with 0 timeouts and to cancel the transfers, we added a code to our libusb implementation. (We could have rewritten the code to use the async calls, but at which point we are re-implementing sync.c anyway, so why not add the function there when we statically link anyway.) That is done here https://github.com/trezor/trezord-go/pull/104\r\n\r\n----\r\n\r\nWell. Since as @alexdupre wrote, it is standard to use system libraries on freebsd, we cannot really add stuff to sync.c there; and the timeout approach causes problems (libusb returns LIBUSB_PIPE error). The best solution to me seems to just not allow session stealing on FreeBSD. (Other solutions would be to rewrite sync.c so it works with system libraries, so it does not use libusbi.h. I am too lazy for that. Other solution would be to statically link libusb as we do on other platforms :D) \r\n\r\nThat makes trezord still mostly usable on freebsd - just that \"session stealing\" is not working properly so when user has more tabs, one can block communication for other tabs. That makes it sub-obtimal but still usable - user sees that there is communication in some other tab so he can go and close all other tabs manually.\r\n\r\n----\r\n\r\nNow I added function to trezord.go that detects whether to allow session stealing (only when the OS is freebsd) and slightly refactored whether to use both webusb.go and hid.go (when not freebsd - or linux, see below)\r\n\r\nWhen I refactored, I realized we can also use libusb for the HID devices on linux - since hidapi uses libusb on linux anyway - so I added a code that uses libusb on linux instead, instead of using hidapi which uses libusb.\r\n\r\nAt which point I realized we can probaby rename webusb.go to libusb.go and the class from WebUSB to LibUSB, because it makes more sense, so I did that.\r\n\r\n....and that is the PR below. :D "},"comment":{"url":"https://api.github.com/repos/trezor/trezord-go/issues/comments/426336911","html_url":"https://github.com/trezor/trezord-go/pull/108#issuecomment-426336911","issue_url":"https://api.github.com/repos/trezor/trezord-go/issues/108","id":426336911,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNjMzNjkxMQ==","user":{"login":"alexdupre","id":3694371,"node_id":"MDQ6VXNlcjM2OTQzNzE=","avatar_url":"https://avatars1.githubusercontent.com/u/3694371?v=4","gravatar_id":"","url":"https://api.github.com/users/alexdupre","html_url":"https://github.com/alexdupre","followers_url":"https://api.github.com/users/alexdupre/followers","following_url":"https://api.github.com/users/alexdupre/following{/other_user}","gists_url":"https://api.github.com/users/alexdupre/gists{/gist_id}","starred_url":"https://api.github.com/users/alexdupre/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexdupre/subscriptions","organizations_url":"https://api.github.com/users/alexdupre/orgs","repos_url":"https://api.github.com/users/alexdupre/repos","events_url":"https://api.github.com/users/alexdupre/events{/privacy}","received_events_url":"https://api.github.com/users/alexdupre/received_events","type":"User","site_admin":false},"created_at":"2018-10-02T16:19:22Z","updated_at":"2018-10-02T16:19:22Z","author_association":"CONTRIBUTOR","body":"Tested, I'm fine with all commits, session stealing is really a minor feature."}}
{"actor":{"display_login":"alexdupre"}}