{"action":"created","comment":{"url":"https://api.github.com/repos/fnproject/fn/pulls/comments/180918615","pull_request_review_id":111427249,"id":180918615,"diff_hunk":"@@ -57,77 +52,64 @@ func (rp *staticRunnerPool) Runners(call pool.RunnerCall) ([]pool.Runner, error)\n }\n \n func (rp *staticRunnerPool) AddRunner(address string) error {\n-\trp.rMtx.Lock()\n-\tdefer rp.rMtx.Unlock()\n-\n \tr, err := rp.generator(address, rp.runnerCN, rp.pki)\n \tif err != nil {\n \t\tlogrus.WithField(\"runner_addr\", address).Warn(\"Failed to add runner\")\n \t\treturn err\n \t}\n-\trp.runners = append(rp.runners, r)\n+\n+\trp.rMtx.Lock()\n+\n+\tisFound := false\n+\tfor _, r := range rp.runners {\n+\t\tif r.Address() == address {\n+\t\t\tisFound = true\n+\t\t\tbreak\n+\t\t}\n+\t}\n+\tif !isFound {\n+\t\trp.runners = append(rp.runners, r)\n+\t}\n+\n+\trp.rMtx.Unlock()\n \treturn nil\n }\n \n func (rp *staticRunnerPool) RemoveRunner(address string) {\n-\trp.rMtx.Lock()\n-\tdefer rp.rMtx.Unlock()\n \n-\tctx, cancel := context.WithTimeout(context.Background(), staticPoolShutdownTimeout)\n-\tdefer cancel()\n+\tvar toRemove pool.Runner\n+\n+\trp.rMtx.Lock()\n \n \tfor i, r := range rp.runners {\n \t\tif r.Address() == address {\n-\t\t\terr := r.Close(ctx)\n-\t\t\tif err != nil {\n-\t\t\t\tlogrus.WithError(err).WithField(\"runner_addr\", r.Address()).Error(\"Failed to close runner\")\n-\t\t\t}\n-\t\t\t// delete runner from list\n+\t\t\ttoRemove = r\n \t\t\trp.runners = append(rp.runners[:i], rp.runners[i+1:]...)\n-\t\t\treturn\n+\t\t\tbreak\n \t\t}\n \t}\n+\n+\trp.rMtx.Unlock()\n+\n+\tif toRemove != nil {\n+\t\ttoRemove.Close(context.Background())\n+\t}\n }\n \n-// Shutdown blocks waiting for all runners to close, or until ctx is done\n+// Shutdown blocks waiting for all runners to close\n func (rp *staticRunnerPool) Shutdown(ctx context.Context) (e error) {\n+\n+\tvar toRemove []pool.Runner\n+\n \trp.rMtx.Lock()\n-\tdefer rp.rMtx.Unlock()\n \n-\tctx, cancel := context.WithTimeout(context.Background(), staticPoolShutdownTimeout)\n-\tdefer cancel()\n+\ttoRemove = rp.runners[:]","path":"api/agent/static_pool.go","position":90,"original_position":90,"commit_id":"166f10056f41e74cc569ecc10ef5b516b6a559b9","original_commit_id":"166f10056f41e74cc569ecc10ef5b516b6a559b9","user":{"login":"rdallman","id":2935111,"avatar_url":"https://avatars0.githubusercontent.com/u/2935111?v=4","gravatar_id":"","url":"https://api.github.com/users/rdallman","html_url":"https://github.com/rdallman","followers_url":"https://api.github.com/users/rdallman/followers","following_url":"https://api.github.com/users/rdallman/following{/other_user}","gists_url":"https://api.github.com/users/rdallman/gists{/gist_id}","starred_url":"https://api.github.com/users/rdallman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rdallman/subscriptions","organizations_url":"https://api.github.com/users/rdallman/orgs","repos_url":"https://api.github.com/users/rdallman/repos","events_url":"https://api.github.com/users/rdallman/events{/privacy}","received_events_url":"https://api.github.com/users/rdallman/received_events","type":"User","site_admin":false},"body":"nit: could `toRemove :=` here and avoid the 3 lines of var decl, no behavior change.","created_at":"2018-04-11T22:37:44Z","updated_at":"2018-04-11T22:37:44Z","html_url":"https://github.com/fnproject/fn/pull/935#discussion_r180918615","pull_request_url":"https://api.github.com/repos/fnproject/fn/pulls/935","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/fnproject/fn/pulls/comments/180918615"},"html":{"href":"https://github.com/fnproject/fn/pull/935#discussion_r180918615"},"pull_request":{"href":"https://api.github.com/repos/fnproject/fn/pulls/935"}}},"pull_request":{"url":"https://api.github.com/repos/fnproject/fn/pulls/935","id":181032261,"html_url":"https://github.com/fnproject/fn/pull/935","diff_url":"https://github.com/fnproject/fn/pull/935.diff","patch_url":"https://github.com/fnproject/fn/pull/935.patch","issue_url":"https://api.github.com/repos/fnproject/fn/issues/935","number":935,"state":"open","locked":false,"title":"fn: mutex while waiting I/O considered harmful","user":{"login":"skinowski","id":6537562,"avatar_url":"https://avatars2.githubusercontent.com/u/6537562?v=4","gravatar_id":"","url":"https://api.github.com/users/skinowski","html_url":"https://github.com/skinowski","followers_url":"https://api.github.com/users/skinowski/followers","following_url":"https://api.github.com/users/skinowski/following{/other_user}","gists_url":"https://api.github.com/users/skinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/skinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skinowski/subscriptions","organizations_url":"https://api.github.com/users/skinowski/orgs","repos_url":"https://api.github.com/users/skinowski/repos","events_url":"https://api.github.com/users/skinowski/events{/privacy}","received_events_url":"https://api.github.com/users/skinowski/received_events","type":"User","site_admin":false},"body":"*) Removed hold mutex while wait I/O cases these\r\nincluded possible disk I/O and network I/O.\r\n\r\n*) Error/Context Close/Shutdown semantics changed since\r\nthe context timeout and comments were misleading. Close\r\nalways waits for pending gRPC session to complete.\r\nContext usage here was merely 'wait up to x secs to\r\nreport an error' which only logs the error anyway.\r\nInstead, the runner can log the error. And context\r\nstill can be passed around perhaps for future opencensus\r\ninstrumentation.","created_at":"2018-04-11T21:23:29Z","updated_at":"2018-04-11T22:37:44Z","closed_at":null,"merged_at":null,"merge_commit_sha":"41c90d7564a154a78e8264379a2e01d224755c55","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/fnproject/fn/pulls/935/commits","review_comments_url":"https://api.github.com/repos/fnproject/fn/pulls/935/comments","review_comment_url":"https://api.github.com/repos/fnproject/fn/pulls/comments{/number}","comments_url":"https://api.github.com/repos/fnproject/fn/issues/935/comments","statuses_url":"https://api.github.com/repos/fnproject/fn/statuses/166f10056f41e74cc569ecc10ef5b516b6a559b9","head":{"label":"skinowski:lock_hold_io","ref":"lock_hold_io","sha":"166f10056f41e74cc569ecc10ef5b516b6a559b9","user":{"login":"skinowski","id":6537562,"avatar_url":"https://avatars2.githubusercontent.com/u/6537562?v=4","gravatar_id":"","url":"https://api.github.com/users/skinowski","html_url":"https://github.com/skinowski","followers_url":"https://api.github.com/users/skinowski/followers","following_url":"https://api.github.com/users/skinowski/following{/other_user}","gists_url":"https://api.github.com/users/skinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/skinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skinowski/subscriptions","organizations_url":"https://api.github.com/users/skinowski/orgs","repos_url":"https://api.github.com/users/skinowski/repos","events_url":"https://api.github.com/users/skinowski/events{/privacy}","received_events_url":"https://api.github.com/users/skinowski/received_events","type":"User","site_admin":false},"repo":{"id":109311981,"name":"fn","full_name":"skinowski/fn","owner":{"login":"skinowski","id":6537562,"avatar_url":"https://avatars2.githubusercontent.com/u/6537562?v=4","gravatar_id":"","url":"https://api.github.com/users/skinowski","html_url":"https://github.com/skinowski","followers_url":"https://api.github.com/users/skinowski/followers","following_url":"https://api.github.com/users/skinowski/following{/other_user}","gists_url":"https://api.github.com/users/skinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/skinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skinowski/subscriptions","organizations_url":"https://api.github.com/users/skinowski/orgs","repos_url":"https://api.github.com/users/skinowski/repos","events_url":"https://api.github.com/users/skinowski/events{/privacy}","received_events_url":"https://api.github.com/users/skinowski/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/skinowski/fn","description":"The container native, cloud agnostic serverless platform.","fork":true,"url":"https://api.github.com/repos/skinowski/fn","forks_url":"https://api.github.com/repos/skinowski/fn/forks","keys_url":"https://api.github.com/repos/skinowski/fn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/skinowski/fn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/skinowski/fn/teams","hooks_url":"https://api.github.com/repos/skinowski/fn/hooks","issue_events_url":"https://api.github.com/repos/skinowski/fn/issues/events{/number}","events_url":"https://api.github.com/repos/skinowski/fn/events","assignees_url":"https://api.github.com/repos/skinowski/fn/assignees{/user}","branches_url":"https://api.github.com/repos/skinowski/fn/branches{/branch}","tags_url":"https://api.github.com/repos/skinowski/fn/tags","blobs_url":"https://api.github.com/repos/skinowski/fn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/skinowski/fn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/skinowski/fn/git/refs{/sha}","trees_url":"https://api.github.com/repos/skinowski/fn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/skinowski/fn/statuses/{sha}","languages_url":"https://api.github.com/repos/skinowski/fn/languages","stargazers_url":"https://api.github.com/repos/skinowski/fn/stargazers","contributors_url":"https://api.github.com/repos/skinowski/fn/contributors","subscribers_url":"https://api.github.com/repos/skinowski/fn/subscribers","subscription_url":"https://api.github.com/repos/skinowski/fn/subscription","commits_url":"https://api.github.com/repos/skinowski/fn/commits{/sha}","git_commits_url":"https://api.github.com/repos/skinowski/fn/git/commits{/sha}","comments_url":"https://api.github.com/repos/skinowski/fn/comments{/number}","issue_comment_url":"https://api.github.com/repos/skinowski/fn/issues/comments{/number}","contents_url":"https://api.github.com/repos/skinowski/fn/contents/{+path}","compare_url":"https://api.github.com/repos/skinowski/fn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/skinowski/fn/merges","archive_url":"https://api.github.com/repos/skinowski/fn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/skinowski/fn/downloads","issues_url":"https://api.github.com/repos/skinowski/fn/issues{/number}","pulls_url":"https://api.github.com/repos/skinowski/fn/pulls{/number}","milestones_url":"https://api.github.com/repos/skinowski/fn/milestones{/number}","notifications_url":"https://api.github.com/repos/skinowski/fn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/skinowski/fn/labels{/name}","releases_url":"https://api.github.com/repos/skinowski/fn/releases{/id}","deployments_url":"https://api.github.com/repos/skinowski/fn/deployments","created_at":"2017-11-02T19:51:42Z","updated_at":"2018-04-11T21:24:23Z","pushed_at":"2018-04-11T21:26:01Z","git_url":"git://github.com/skinowski/fn.git","ssh_url":"git@github.com:skinowski/fn.git","clone_url":"https://github.com/skinowski/fn.git","svn_url":"https://github.com/skinowski/fn","homepage":"http://fnproject.io","size":133955,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"fnproject:master","ref":"master","sha":"9b86e3626e5bd0ce8fcd68e87f10f6f35dc2ca4c","user":{"login":"fnproject","id":30273834,"avatar_url":"https://avatars2.githubusercontent.com/u/30273834?v=4","gravatar_id":"","url":"https://api.github.com/users/fnproject","html_url":"https://github.com/fnproject","followers_url":"https://api.github.com/users/fnproject/followers","following_url":"https://api.github.com/users/fnproject/following{/other_user}","gists_url":"https://api.github.com/users/fnproject/gists{/gist_id}","starred_url":"https://api.github.com/users/fnproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fnproject/subscriptions","organizations_url":"https://api.github.com/users/fnproject/orgs","repos_url":"https://api.github.com/users/fnproject/repos","events_url":"https://api.github.com/users/fnproject/events{/privacy}","received_events_url":"https://api.github.com/users/fnproject/received_events","type":"Organization","site_admin":false},"repo":{"id":97652025,"name":"fn","full_name":"fnproject/fn","owner":{"login":"fnproject","id":30273834,"avatar_url":"https://avatars2.githubusercontent.com/u/30273834?v=4","gravatar_id":"","url":"https://api.github.com/users/fnproject","html_url":"https://github.com/fnproject","followers_url":"https://api.github.com/users/fnproject/followers","following_url":"https://api.github.com/users/fnproject/following{/other_user}","gists_url":"https://api.github.com/users/fnproject/gists{/gist_id}","starred_url":"https://api.github.com/users/fnproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fnproject/subscriptions","organizations_url":"https://api.github.com/users/fnproject/orgs","repos_url":"https://api.github.com/users/fnproject/repos","events_url":"https://api.github.com/users/fnproject/events{/privacy}","received_events_url":"https://api.github.com/users/fnproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/fnproject/fn","description":"The container native, cloud agnostic serverless platform.","fork":false,"url":"https://api.github.com/repos/fnproject/fn","forks_url":"https://api.github.com/repos/fnproject/fn/forks","keys_url":"https://api.github.com/repos/fnproject/fn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/fnproject/fn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/fnproject/fn/teams","hooks_url":"https://api.github.com/repos/fnproject/fn/hooks","issue_events_url":"https://api.github.com/repos/fnproject/fn/issues/events{/number}","events_url":"https://api.github.com/repos/fnproject/fn/events","assignees_url":"https://api.github.com/repos/fnproject/fn/assignees{/user}","branches_url":"https://api.github.com/repos/fnproject/fn/branches{/branch}","tags_url":"https://api.github.com/repos/fnproject/fn/tags","blobs_url":"https://api.github.com/repos/fnproject/fn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/fnproject/fn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/fnproject/fn/git/refs{/sha}","trees_url":"https://api.github.com/repos/fnproject/fn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/fnproject/fn/statuses/{sha}","languages_url":"https://api.github.com/repos/fnproject/fn/languages","stargazers_url":"https://api.github.com/repos/fnproject/fn/stargazers","contributors_url":"https://api.github.com/repos/fnproject/fn/contributors","subscribers_url":"https://api.github.com/repos/fnproject/fn/subscribers","subscription_url":"https://api.github.com/repos/fnproject/fn/subscription","commits_url":"https://api.github.com/repos/fnproject/fn/commits{/sha}","git_commits_url":"https://api.github.com/repos/fnproject/fn/git/commits{/sha}","comments_url":"https://api.github.com/repos/fnproject/fn/comments{/number}","issue_comment_url":"https://api.github.com/repos/fnproject/fn/issues/comments{/number}","contents_url":"https://api.github.com/repos/fnproject/fn/contents/{+path}","compare_url":"https://api.github.com/repos/fnproject/fn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/fnproject/fn/merges","archive_url":"https://api.github.com/repos/fnproject/fn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/fnproject/fn/downloads","issues_url":"https://api.github.com/repos/fnproject/fn/issues{/number}","pulls_url":"https://api.github.com/repos/fnproject/fn/pulls{/number}","milestones_url":"https://api.github.com/repos/fnproject/fn/milestones{/number}","notifications_url":"https://api.github.com/repos/fnproject/fn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/fnproject/fn/labels{/name}","releases_url":"https://api.github.com/repos/fnproject/fn/releases{/id}","deployments_url":"https://api.github.com/repos/fnproject/fn/deployments","created_at":"2017-07-18T23:22:46Z","updated_at":"2018-04-11T20:51:42Z","pushed_at":"2018-04-11T21:26:03Z","git_url":"git://github.com/fnproject/fn.git","ssh_url":"git@github.com:fnproject/fn.git","clone_url":"https://github.com/fnproject/fn.git","svn_url":"https://github.com/fnproject/fn","homepage":"http://fnproject.io","size":152206,"stargazers_count":2607,"watchers_count":2607,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":181,"mirror_url":null,"archived":false,"open_issues_count":106,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":181,"open_issues":106,"watchers":2607,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/fnproject/fn/pulls/935"},"html":{"href":"https://github.com/fnproject/fn/pull/935"},"issue":{"href":"https://api.github.com/repos/fnproject/fn/issues/935"},"comments":{"href":"https://api.github.com/repos/fnproject/fn/issues/935/comments"},"review_comments":{"href":"https://api.github.com/repos/fnproject/fn/pulls/935/comments"},"review_comment":{"href":"https://api.github.com/repos/fnproject/fn/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/fnproject/fn/pulls/935/commits"},"statuses":{"href":"https://api.github.com/repos/fnproject/fn/statuses/166f10056f41e74cc569ecc10ef5b516b6a559b9"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"rdallman"}}