{"action":"created","comment":{"url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/comments/43797164","id":43797164,"diff_hunk":"@@ -67,21 +69,28 @@ func (m *Mutex) commonChecks() error {\n // Lock attempts to acquire the mutex lock, and wil block if it is\n // already locked.\n func (m *Mutex) Lock() error {\n-\tif err := m.commonChecks(); err != nil {\n-\t\treturn err\n-\t}\n-\n-\treturn m.lock.Lock(true)\n+\treturn m.lock(true)\n }\n \n // TryLock attempts to acquire the mutex lock. If it is already locked,\n // it will return ErrAlreadyLocked.\n func (m *Mutex) TryLock() error {\n+\treturn m.lock(false)\n+}\n+\n+func (m *Mutex) lock(blocking bool) error {\n \tif err := m.commonChecks(); err != nil {","path":"priv/priv.go","position":42,"original_position":42,"commit_id":"403825b78e1d5dda1d72528d7ba512ef23b7b21d","original_commit_id":"403825b78e1d5dda1d72528d7ba512ef23b7b21d","user":{"login":"chipaca","id":1543430,"avatar_url":"https://avatars.githubusercontent.com/u/1543430?v=3","gravatar_id":"","url":"https://api.github.com/users/chipaca","html_url":"https://github.com/chipaca","followers_url":"https://api.github.com/users/chipaca/followers","following_url":"https://api.github.com/users/chipaca/following{/other_user}","gists_url":"https://api.github.com/users/chipaca/gists{/gist_id}","starred_url":"https://api.github.com/users/chipaca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chipaca/subscriptions","organizations_url":"https://api.github.com/users/chipaca/orgs","repos_url":"https://api.github.com/users/chipaca/repos","events_url":"https://api.github.com/users/chipaca/events{/privacy}","received_events_url":"https://api.github.com/users/chipaca/received_events","type":"User","site_admin":false},"body":"I think a later branch could take priv and split it in two: the file lock on the one hand, and the privilege check on the other. I'm not sure why they were conflated, but at least the implementations are separate enough that separating the packages should be trivial.","created_at":"2015-11-03T19:45:34Z","updated_at":"2015-11-03T19:45:34Z","html_url":"https://github.com/ubuntu-core/snappy/pull/42#discussion_r43797164","pull_request_url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42","_links":{"self":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/comments/43797164"},"html":{"href":"https://github.com/ubuntu-core/snappy/pull/42#discussion_r43797164"},"pull_request":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42"}}},"pull_request":{"url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42","id":49278689,"html_url":"https://github.com/ubuntu-core/snappy/pull/42","diff_url":"https://github.com/ubuntu-core/snappy/pull/42.diff","patch_url":"https://github.com/ubuntu-core/snappy/pull/42.patch","issue_url":"https://api.github.com/repos/ubuntu-core/snappy/issues/42","number":42,"state":"open","locked":false,"title":"rework filelock to make it less surprising wrt flock(2) semantics","user":{"login":"chipaca","id":1543430,"avatar_url":"https://avatars.githubusercontent.com/u/1543430?v=3","gravatar_id":"","url":"https://api.github.com/users/chipaca","html_url":"https://github.com/chipaca","followers_url":"https://api.github.com/users/chipaca/followers","following_url":"https://api.github.com/users/chipaca/following{/other_user}","gists_url":"https://api.github.com/users/chipaca/gists{/gist_id}","starred_url":"https://api.github.com/users/chipaca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chipaca/subscriptions","organizations_url":"https://api.github.com/users/chipaca/orgs","repos_url":"https://api.github.com/users/chipaca/repos","events_url":"https://api.github.com/users/chipaca/events{/privacy}","received_events_url":"https://api.github.com/users/chipaca/received_events","type":"User","site_admin":false},"body":"flock(2)'s semantics are such that calling flock twice on the same file descriptor releases and reacquires the lock non-atomically, so the old FileLock that presented a Lock/Unlock interface was bad: locking the same file object twice was never what is wanted. Worse, Unlock deleted the file*, so if you had a different instance of FileLock waiting on that, and Unlocked it, and then a third instance of the FileLock was instantiated, the third and second would run concurrently because the files were different.\r\n\r\n* it deleted the file without the lock held, which probably introduces even more fun races that I haven't stopped to think about.\r\n\r\nAnyway, I reworked the filelock, so that now it's actually a much more direct call to flock(), to the point that the thing exposed is just the file descriptor.\r\n\r\nThis is probably the only thing I'm not sure about in the code, wrt design: making it a file descriptor makes the code sillyly simple, but the “null value” of a file descriptor is conventionally -1, and that leaks out. I don't like that much, but I dislike having a struct just to hide that a smidgen more.\r\n\r\nThis *also* fixes a bug that was hard to reproduce locally, but consistently got travis upset, involving a race in TestWithPrivMutexErrOnLockHeld where the mutex was held in a goroutine but the test checked that a function ran with the mutex held before that goroutine got around to running the function. Forced synchronization with a flag channel.","created_at":"2015-10-30T14:53:27Z","updated_at":"2015-11-03T19:45:34Z","closed_at":null,"merged_at":null,"merge_commit_sha":"18b02f32af4245466a25175ed3668c38d1032d7e","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42/commits","review_comments_url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42/comments","review_comment_url":"https://api.github.com/repos/ubuntu-core/snappy/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ubuntu-core/snappy/issues/42/comments","statuses_url":"https://api.github.com/repos/ubuntu-core/snappy/statuses/403825b78e1d5dda1d72528d7ba512ef23b7b21d","head":{"label":"chipaca:priv-flock-rework","ref":"priv-flock-rework","sha":"403825b78e1d5dda1d72528d7ba512ef23b7b21d","user":{"login":"chipaca","id":1543430,"avatar_url":"https://avatars.githubusercontent.com/u/1543430?v=3","gravatar_id":"","url":"https://api.github.com/users/chipaca","html_url":"https://github.com/chipaca","followers_url":"https://api.github.com/users/chipaca/followers","following_url":"https://api.github.com/users/chipaca/following{/other_user}","gists_url":"https://api.github.com/users/chipaca/gists{/gist_id}","starred_url":"https://api.github.com/users/chipaca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chipaca/subscriptions","organizations_url":"https://api.github.com/users/chipaca/orgs","repos_url":"https://api.github.com/users/chipaca/repos","events_url":"https://api.github.com/users/chipaca/events{/privacy}","received_events_url":"https://api.github.com/users/chipaca/received_events","type":"User","site_admin":false},"repo":{"id":45061368,"name":"snappy","full_name":"chipaca/snappy","owner":{"login":"chipaca","id":1543430,"avatar_url":"https://avatars.githubusercontent.com/u/1543430?v=3","gravatar_id":"","url":"https://api.github.com/users/chipaca","html_url":"https://github.com/chipaca","followers_url":"https://api.github.com/users/chipaca/followers","following_url":"https://api.github.com/users/chipaca/following{/other_user}","gists_url":"https://api.github.com/users/chipaca/gists{/gist_id}","starred_url":"https://api.github.com/users/chipaca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chipaca/subscriptions","organizations_url":"https://api.github.com/users/chipaca/orgs","repos_url":"https://api.github.com/users/chipaca/repos","events_url":"https://api.github.com/users/chipaca/events{/privacy}","received_events_url":"https://api.github.com/users/chipaca/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/chipaca/snappy","description":"Snappy is the software at the heart of Ubuntu Core.","fork":true,"url":"https://api.github.com/repos/chipaca/snappy","forks_url":"https://api.github.com/repos/chipaca/snappy/forks","keys_url":"https://api.github.com/repos/chipaca/snappy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/chipaca/snappy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/chipaca/snappy/teams","hooks_url":"https://api.github.com/repos/chipaca/snappy/hooks","issue_events_url":"https://api.github.com/repos/chipaca/snappy/issues/events{/number}","events_url":"https://api.github.com/repos/chipaca/snappy/events","assignees_url":"https://api.github.com/repos/chipaca/snappy/assignees{/user}","branches_url":"https://api.github.com/repos/chipaca/snappy/branches{/branch}","tags_url":"https://api.github.com/repos/chipaca/snappy/tags","blobs_url":"https://api.github.com/repos/chipaca/snappy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/chipaca/snappy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/chipaca/snappy/git/refs{/sha}","trees_url":"https://api.github.com/repos/chipaca/snappy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/chipaca/snappy/statuses/{sha}","languages_url":"https://api.github.com/repos/chipaca/snappy/languages","stargazers_url":"https://api.github.com/repos/chipaca/snappy/stargazers","contributors_url":"https://api.github.com/repos/chipaca/snappy/contributors","subscribers_url":"https://api.github.com/repos/chipaca/snappy/subscribers","subscription_url":"https://api.github.com/repos/chipaca/snappy/subscription","commits_url":"https://api.github.com/repos/chipaca/snappy/commits{/sha}","git_commits_url":"https://api.github.com/repos/chipaca/snappy/git/commits{/sha}","comments_url":"https://api.github.com/repos/chipaca/snappy/comments{/number}","issue_comment_url":"https://api.github.com/repos/chipaca/snappy/issues/comments{/number}","contents_url":"https://api.github.com/repos/chipaca/snappy/contents/{+path}","compare_url":"https://api.github.com/repos/chipaca/snappy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/chipaca/snappy/merges","archive_url":"https://api.github.com/repos/chipaca/snappy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/chipaca/snappy/downloads","issues_url":"https://api.github.com/repos/chipaca/snappy/issues{/number}","pulls_url":"https://api.github.com/repos/chipaca/snappy/pulls{/number}","milestones_url":"https://api.github.com/repos/chipaca/snappy/milestones{/number}","notifications_url":"https://api.github.com/repos/chipaca/snappy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/chipaca/snappy/labels{/name}","releases_url":"https://api.github.com/repos/chipaca/snappy/releases{/id}","created_at":"2015-10-27T18:26:51Z","updated_at":"2015-10-27T18:26:55Z","pushed_at":"2015-11-03T19:40:31Z","git_url":"git://github.com/chipaca/snappy.git","ssh_url":"git@github.com:chipaca/snappy.git","clone_url":"https://github.com/chipaca/snappy.git","svn_url":"https://github.com/chipaca/snappy","homepage":null,"size":23336,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"ubuntu-core:master","ref":"master","sha":"97cf364dff352152edd4f37b66e819b698c3ad6b","user":{"login":"ubuntu-core","id":15022473,"avatar_url":"https://avatars.githubusercontent.com/u/15022473?v=3","gravatar_id":"","url":"https://api.github.com/users/ubuntu-core","html_url":"https://github.com/ubuntu-core","followers_url":"https://api.github.com/users/ubuntu-core/followers","following_url":"https://api.github.com/users/ubuntu-core/following{/other_user}","gists_url":"https://api.github.com/users/ubuntu-core/gists{/gist_id}","starred_url":"https://api.github.com/users/ubuntu-core/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ubuntu-core/subscriptions","organizations_url":"https://api.github.com/users/ubuntu-core/orgs","repos_url":"https://api.github.com/users/ubuntu-core/repos","events_url":"https://api.github.com/users/ubuntu-core/events{/privacy}","received_events_url":"https://api.github.com/users/ubuntu-core/received_events","type":"Organization","site_admin":false},"repo":{"id":44672744,"name":"snappy","full_name":"ubuntu-core/snappy","owner":{"login":"ubuntu-core","id":15022473,"avatar_url":"https://avatars.githubusercontent.com/u/15022473?v=3","gravatar_id":"","url":"https://api.github.com/users/ubuntu-core","html_url":"https://github.com/ubuntu-core","followers_url":"https://api.github.com/users/ubuntu-core/followers","following_url":"https://api.github.com/users/ubuntu-core/following{/other_user}","gists_url":"https://api.github.com/users/ubuntu-core/gists{/gist_id}","starred_url":"https://api.github.com/users/ubuntu-core/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ubuntu-core/subscriptions","organizations_url":"https://api.github.com/users/ubuntu-core/orgs","repos_url":"https://api.github.com/users/ubuntu-core/repos","events_url":"https://api.github.com/users/ubuntu-core/events{/privacy}","received_events_url":"https://api.github.com/users/ubuntu-core/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ubuntu-core/snappy","description":"Snappy is the software at the heart of Ubuntu Core.","fork":false,"url":"https://api.github.com/repos/ubuntu-core/snappy","forks_url":"https://api.github.com/repos/ubuntu-core/snappy/forks","keys_url":"https://api.github.com/repos/ubuntu-core/snappy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ubuntu-core/snappy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ubuntu-core/snappy/teams","hooks_url":"https://api.github.com/repos/ubuntu-core/snappy/hooks","issue_events_url":"https://api.github.com/repos/ubuntu-core/snappy/issues/events{/number}","events_url":"https://api.github.com/repos/ubuntu-core/snappy/events","assignees_url":"https://api.github.com/repos/ubuntu-core/snappy/assignees{/user}","branches_url":"https://api.github.com/repos/ubuntu-core/snappy/branches{/branch}","tags_url":"https://api.github.com/repos/ubuntu-core/snappy/tags","blobs_url":"https://api.github.com/repos/ubuntu-core/snappy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ubuntu-core/snappy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ubuntu-core/snappy/git/refs{/sha}","trees_url":"https://api.github.com/repos/ubuntu-core/snappy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ubuntu-core/snappy/statuses/{sha}","languages_url":"https://api.github.com/repos/ubuntu-core/snappy/languages","stargazers_url":"https://api.github.com/repos/ubuntu-core/snappy/stargazers","contributors_url":"https://api.github.com/repos/ubuntu-core/snappy/contributors","subscribers_url":"https://api.github.com/repos/ubuntu-core/snappy/subscribers","subscription_url":"https://api.github.com/repos/ubuntu-core/snappy/subscription","commits_url":"https://api.github.com/repos/ubuntu-core/snappy/commits{/sha}","git_commits_url":"https://api.github.com/repos/ubuntu-core/snappy/git/commits{/sha}","comments_url":"https://api.github.com/repos/ubuntu-core/snappy/comments{/number}","issue_comment_url":"https://api.github.com/repos/ubuntu-core/snappy/issues/comments{/number}","contents_url":"https://api.github.com/repos/ubuntu-core/snappy/contents/{+path}","compare_url":"https://api.github.com/repos/ubuntu-core/snappy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ubuntu-core/snappy/merges","archive_url":"https://api.github.com/repos/ubuntu-core/snappy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ubuntu-core/snappy/downloads","issues_url":"https://api.github.com/repos/ubuntu-core/snappy/issues{/number}","pulls_url":"https://api.github.com/repos/ubuntu-core/snappy/pulls{/number}","milestones_url":"https://api.github.com/repos/ubuntu-core/snappy/milestones{/number}","notifications_url":"https://api.github.com/repos/ubuntu-core/snappy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ubuntu-core/snappy/labels{/name}","releases_url":"https://api.github.com/repos/ubuntu-core/snappy/releases{/id}","created_at":"2015-10-21T11:51:46Z","updated_at":"2015-11-03T15:37:07Z","pushed_at":"2015-11-03T19:40:31Z","git_url":"git://github.com/ubuntu-core/snappy.git","ssh_url":"git@github.com:ubuntu-core/snappy.git","clone_url":"https://github.com/ubuntu-core/snappy.git","svn_url":"https://github.com/ubuntu-core/snappy","homepage":null,"size":24176,"stargazers_count":28,"watchers_count":28,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":10,"mirror_url":null,"open_issues_count":17,"forks":10,"open_issues":17,"watchers":28,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42"},"html":{"href":"https://github.com/ubuntu-core/snappy/pull/42"},"issue":{"href":"https://api.github.com/repos/ubuntu-core/snappy/issues/42"},"comments":{"href":"https://api.github.com/repos/ubuntu-core/snappy/issues/42/comments"},"review_comments":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42/comments"},"review_comment":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ubuntu-core/snappy/pulls/42/commits"},"statuses":{"href":"https://api.github.com/repos/ubuntu-core/snappy/statuses/403825b78e1d5dda1d72528d7ba512ef23b7b21d"}}}}