{"action":"created","issue":{"url":"https://api.github.com/repos/Jeffail/leaps/issues/9","repository_url":"https://api.github.com/repos/Jeffail/leaps","labels_url":"https://api.github.com/repos/Jeffail/leaps/issues/9/labels{/name}","comments_url":"https://api.github.com/repos/Jeffail/leaps/issues/9/comments","events_url":"https://api.github.com/repos/Jeffail/leaps/issues/9/events","html_url":"https://github.com/Jeffail/leaps/issues/9","id":109695391,"number":9,"title":"binderMutex shouldn't have I/O in critical sections","user":{"login":"jonasfj","id":149732,"avatar_url":"https://avatars.githubusercontent.com/u/149732?v=3","gravatar_id":"","url":"https://api.github.com/users/jonasfj","html_url":"https://github.com/jonasfj","followers_url":"https://api.github.com/users/jonasfj/followers","following_url":"https://api.github.com/users/jonasfj/following{/other_user}","gists_url":"https://api.github.com/users/jonasfj/gists{/gist_id}","starred_url":"https://api.github.com/users/jonasfj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonasfj/subscriptions","organizations_url":"https://api.github.com/users/jonasfj/orgs","repos_url":"https://api.github.com/users/jonasfj/repos","events_url":"https://api.github.com/users/jonasfj/events{/privacy}","received_events_url":"https://api.github.com/users/jonasfj/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2015-10-04T18:31:22Z","updated_at":"2016-06-26T16:57:39Z","closed_at":null,"body":"We critically should fix the `binderMutex` lock... The critical section covers `NewBinder` which does a `store.Fetch`... This is problematic as people can't join/create documents while we wait for a single document to be loaded...\r\n\r\nI looked at it, and I think we should just make an `Abort` method on binder that aborts the loop without saving anything... I just couldn't quiet figure out how to do it... @Jeffail, If you can give a hint on how to make an `Abort()` function that doesn't save the bind, but cleans up all resources, I can do the rest :)\r\n(I'm still a bit new to golang)\r\n\r\nWith the abort function we can do as follows:\r\n 1) Lock `binderMutex`\r\n 2) Look in `openBinders` for binder\r\n 3) Unlock `binderMutex`\r\n 4) Create binder: `b := NewBinder(...)`\r\n 5) Lock `binderMutex`\r\n 6) If `binder := openBinders[id]` exists, call `b.Abort()`, else set `binder := openBinders[id] = b`\r\n 7) Unlock `binderMutex`\r\n 8) `return binder.Subscribe(...`\r\n\r\nSure, sometime we may create a binder without any clients at the same time as another client creates the same binder... But the binder that is added to `openBinders[id]` is the one both will use, and the other binding will be aborted before any clients are subscribed to it..."},"comment":{"url":"https://api.github.com/repos/Jeffail/leaps/issues/comments/228610840","html_url":"https://github.com/Jeffail/leaps/issues/9#issuecomment-228610840","issue_url":"https://api.github.com/repos/Jeffail/leaps/issues/9","id":228610840,"user":{"login":"lennart","id":28988,"avatar_url":"https://avatars.githubusercontent.com/u/28988?v=3","gravatar_id":"","url":"https://api.github.com/users/lennart","html_url":"https://github.com/lennart","followers_url":"https://api.github.com/users/lennart/followers","following_url":"https://api.github.com/users/lennart/following{/other_user}","gists_url":"https://api.github.com/users/lennart/gists{/gist_id}","starred_url":"https://api.github.com/users/lennart/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lennart/subscriptions","organizations_url":"https://api.github.com/users/lennart/orgs","repos_url":"https://api.github.com/users/lennart/repos","events_url":"https://api.github.com/users/lennart/events{/privacy}","received_events_url":"https://api.github.com/users/lennart/received_events","type":"User","site_admin":false},"created_at":"2016-06-26T16:57:39Z","updated_at":"2016-06-26T16:57:39Z","body":"I get `panic` seemingly random, from L 411 in `binder.go` (slice bounds out of range) which seem somewhat weird, since indices are read from the binder's list of clients that is sliced. I wonder if this is also related to the race condition you talk about, since multiple places change `b.clients` (add or remove) which (althoug I am not sure about this) could lead to the above. \r\n\r\nIf I read the source around the panicking line, maybe it would be better to somehow identify the _to be kicked_ client by something else than the volatile indices (I guess they are simply invalid by the time of timeout)"}}
{"actor":{"display_login":"lennart"}}