{"action":"created","issue":{"url":"https://api.github.com/repos/ceph/ceph/issues/24701","repository_url":"https://api.github.com/repos/ceph/ceph","labels_url":"https://api.github.com/repos/ceph/ceph/issues/24701/labels{/name}","comments_url":"https://api.github.com/repos/ceph/ceph/issues/24701/comments","events_url":"https://api.github.com/repos/ceph/ceph/issues/24701/events","html_url":"https://github.com/ceph/ceph/pull/24701","id":372667261,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0Nzg5NzQ2","number":24701,"title":"os/bluestore: fix race between SharedBlobSet::lookup and SharedBlob::put","user":{"login":"liewegas","id":433031,"node_id":"MDQ6VXNlcjQzMzAzMQ==","avatar_url":"https://avatars1.githubusercontent.com/u/433031?v=4","gravatar_id":"","url":"https://api.github.com/users/liewegas","html_url":"https://github.com/liewegas","followers_url":"https://api.github.com/users/liewegas/followers","following_url":"https://api.github.com/users/liewegas/following{/other_user}","gists_url":"https://api.github.com/users/liewegas/gists{/gist_id}","starred_url":"https://api.github.com/users/liewegas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liewegas/subscriptions","organizations_url":"https://api.github.com/users/liewegas/orgs","repos_url":"https://api.github.com/users/liewegas/repos","events_url":"https://api.github.com/users/liewegas/events{/privacy}","received_events_url":"https://api.github.com/users/liewegas/received_events","type":"User","site_admin":false},"labels":[{"id":385981601,"node_id":"MDU6TGFiZWwzODU5ODE2MDE=","url":"https://api.github.com/repos/ceph/ceph/labels/bluestore","name":"bluestore","color":"0066ff","default":false},{"id":134774049,"node_id":"MDU6TGFiZWwxMzQ3NzQwNDk=","url":"https://api.github.com/repos/ceph/ceph/labels/bug%20fix","name":"bug fix","color":"ac0000","default":false}],"state":"open","locked":false,"assignee":{"login":"rzarzynski","id":1891217,"node_id":"MDQ6VXNlcjE4OTEyMTc=","avatar_url":"https://avatars2.githubusercontent.com/u/1891217?v=4","gravatar_id":"","url":"https://api.github.com/users/rzarzynski","html_url":"https://github.com/rzarzynski","followers_url":"https://api.github.com/users/rzarzynski/followers","following_url":"https://api.github.com/users/rzarzynski/following{/other_user}","gists_url":"https://api.github.com/users/rzarzynski/gists{/gist_id}","starred_url":"https://api.github.com/users/rzarzynski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rzarzynski/subscriptions","organizations_url":"https://api.github.com/users/rzarzynski/orgs","repos_url":"https://api.github.com/users/rzarzynski/repos","events_url":"https://api.github.com/users/rzarzynski/events{/privacy}","received_events_url":"https://api.github.com/users/rzarzynski/received_events","type":"User","site_admin":false},"assignees":[{"login":"rzarzynski","id":1891217,"node_id":"MDQ6VXNlcjE4OTEyMTc=","avatar_url":"https://avatars2.githubusercontent.com/u/1891217?v=4","gravatar_id":"","url":"https://api.github.com/users/rzarzynski","html_url":"https://github.com/rzarzynski","followers_url":"https://api.github.com/users/rzarzynski/followers","following_url":"https://api.github.com/users/rzarzynski/following{/other_user}","gists_url":"https://api.github.com/users/rzarzynski/gists{/gist_id}","starred_url":"https://api.github.com/users/rzarzynski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rzarzynski/subscriptions","organizations_url":"https://api.github.com/users/rzarzynski/orgs","repos_url":"https://api.github.com/users/rzarzynski/repos","events_url":"https://api.github.com/users/rzarzynski/events{/privacy}","received_events_url":"https://api.github.com/users/rzarzynski/received_events","type":"User","site_admin":false},{"login":"ifed01","id":7138176,"node_id":"MDQ6VXNlcjcxMzgxNzY=","avatar_url":"https://avatars2.githubusercontent.com/u/7138176?v=4","gravatar_id":"","url":"https://api.github.com/users/ifed01","html_url":"https://github.com/ifed01","followers_url":"https://api.github.com/users/ifed01/followers","following_url":"https://api.github.com/users/ifed01/following{/other_user}","gists_url":"https://api.github.com/users/ifed01/gists{/gist_id}","starred_url":"https://api.github.com/users/ifed01/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ifed01/subscriptions","organizations_url":"https://api.github.com/users/ifed01/orgs","repos_url":"https://api.github.com/users/ifed01/repos","events_url":"https://api.github.com/users/ifed01/events{/privacy}","received_events_url":"https://api.github.com/users/ifed01/received_events","type":"User","site_admin":false}],"milestone":null,"comments":0,"created_at":"2018-10-22T19:39:15Z","updated_at":"2018-10-22T19:41:17Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/ceph/ceph/pulls/24701","html_url":"https://github.com/ceph/ceph/pull/24701","diff_url":"https://github.com/ceph/ceph/pull/24701.diff","patch_url":"https://github.com/ceph/ceph/pull/24701.patch"},"body":"```\r\nA                             B\r\nSharedBlobSet::lookup()\r\n  takes lock\r\n  nref is not 0\r\n                              SharedBlob::put()\r\n                                --nref\r\nreturns SharedBlobRef,\r\n  ++nref\r\n                                takes cache lock\r\n                                SharedBlobSet::remove\r\n                                  takes lock\r\n                                  removes\r\n                                deletes SharedBlob\r\n```\r\n-> A ends up with a ref to deleted SharedBlob\r\n\r\nFix by verifying that nref is still zero in SharedBlobSet::remove(),\r\nwhile we are holding the SharedBlobSet::lock.  The lock ensures that we\r\nhave increased the ref for the lookup before entering remove, so we can\r\nverify that nref is still zero before removing it.  If not, we have\r\nraced, and put() bails out and does nothing.\r\n\r\nFixes: http://tracker.ceph.com/issues/36526\r\nSigned-off-by: Sage Weil <sage@redhat.com>"},"comment":{"url":"https://api.github.com/repos/ceph/ceph/issues/comments/431953853","html_url":"https://github.com/ceph/ceph/pull/24701#issuecomment-431953853","issue_url":"https://api.github.com/repos/ceph/ceph/issues/24701","id":431953853,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMTk1Mzg1Mw==","user":{"login":"liewegas","id":433031,"node_id":"MDQ6VXNlcjQzMzAzMQ==","avatar_url":"https://avatars1.githubusercontent.com/u/433031?v=4","gravatar_id":"","url":"https://api.github.com/users/liewegas","html_url":"https://github.com/liewegas","followers_url":"https://api.github.com/users/liewegas/followers","following_url":"https://api.github.com/users/liewegas/following{/other_user}","gists_url":"https://api.github.com/users/liewegas/gists{/gist_id}","starred_url":"https://api.github.com/users/liewegas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liewegas/subscriptions","organizations_url":"https://api.github.com/users/liewegas/orgs","repos_url":"https://api.github.com/users/liewegas/repos","events_url":"https://api.github.com/users/liewegas/events{/privacy}","received_events_url":"https://api.github.com/users/liewegas/received_events","type":"User","site_admin":false},"created_at":"2018-10-22T19:41:17Z","updated_at":"2018-10-22T19:41:17Z","author_association":"MEMBER","body":"@adamemerson just want to confirm the behavior here:\r\n```\r\n    SharedBlobRef lookup(uint64_t sbid) {\r\n      std::lock_guard l(lock);\r\n      auto p = sb_map.find(sbid);\r\n      if (p == sb_map.end() ||\r\n\t  p->second->nref == 0) {\r\n        return nullptr;\r\n      }\r\n      return p->second;\r\n    }\r\n```\r\nThe SharedBlobRef return value is constructed, and nref incremented, before the lock_guard unwinds and lock is dropped, right?"}}
{"actor":{"display_login":"liewegas"}}