{"action":"created","comment":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/77384672","id":77384672,"diff_hunk":"@@ -1193,13 +1194,27 @@ tbm_create_stream_node(HashBitmap *tbm)\n     is->upd_instrument = tbm_stream_upd_instrument;\n \n \top->tbm = tbm;\n+\top->tofree = true;\n \top->entry = NULL;\n \n \tis->opaque = (void *)op;\n \n \treturn is;\n }\n \n+StreamNode *\n+tbm_create_stream_node_ref(HashBitmap *tbm)\n+{\n+\tIndexStream* sn = tbm_create_stream_node(tbm);\n+\t/*\n+\t * Do not take ownership of the HashBitmap.\n+\t */\n+\tHashStreamOpaque *op = (HashStreamOpaque*) sn->opaque;\n+\top->tofree = false;","path":"src/backend/nodes/tidbitmap.c","position":28,"original_position":28,"commit_id":"2addd8803bd046963085970b2b7277a6c22fcb05","original_commit_id":"2addd8803bd046963085970b2b7277a6c22fcb05","user":{"login":"karthijrk","id":5432707,"avatar_url":"https://avatars.githubusercontent.com/u/5432707?v=3","gravatar_id":"","url":"https://api.github.com/users/karthijrk","html_url":"https://github.com/karthijrk","followers_url":"https://api.github.com/users/karthijrk/followers","following_url":"https://api.github.com/users/karthijrk/following{/other_user}","gists_url":"https://api.github.com/users/karthijrk/gists{/gist_id}","starred_url":"https://api.github.com/users/karthijrk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/karthijrk/subscriptions","organizations_url":"https://api.github.com/users/karthijrk/orgs","repos_url":"https://api.github.com/users/karthijrk/repos","events_url":"https://api.github.com/users/karthijrk/events{/privacy}","received_events_url":"https://api.github.com/users/karthijrk/received_events","type":"User","site_admin":false},"body":"I agree, that ICG pass and based on that there may be no issue today. But are we sure by design whoever set the flag as true are the real owner? Also by design are we guaranteed that no one else will de-refer the pointer once the owner deletes it ? \r\n\r\nI don't have full knowledge of the code but just want to make sure there is no potential bug because of this boolean. For e.g. a potential bug could be future code change(if we are not guaranteed by design) could result in deferring the pointer after the owner deletes it. \r\n\r\n ","created_at":"2016-09-02T17:40:44Z","updated_at":"2016-09-02T17:40:44Z","html_url":"https://github.com/greenplum-db/gpdb/pull/1089#discussion_r77384672","pull_request_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089","_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/77384672"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/1089#discussion_r77384672"},"pull_request":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089"}}},"pull_request":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089","id":83365407,"html_url":"https://github.com/greenplum-db/gpdb/pull/1089","diff_url":"https://github.com/greenplum-db/gpdb/pull/1089.diff","patch_url":"https://github.com/greenplum-db/gpdb/pull/1089.patch","issue_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1089","number":1089,"state":"open","locked":false,"title":"Fix double free when using stream bitmaps with hash bitmaps.","user":{"login":"xinzweb","id":723898,"avatar_url":"https://avatars.githubusercontent.com/u/723898?v=3","gravatar_id":"","url":"https://api.github.com/users/xinzweb","html_url":"https://github.com/xinzweb","followers_url":"https://api.github.com/users/xinzweb/followers","following_url":"https://api.github.com/users/xinzweb/following{/other_user}","gists_url":"https://api.github.com/users/xinzweb/gists{/gist_id}","starred_url":"https://api.github.com/users/xinzweb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xinzweb/subscriptions","organizations_url":"https://api.github.com/users/xinzweb/orgs","repos_url":"https://api.github.com/users/xinzweb/repos","events_url":"https://api.github.com/users/xinzweb/events{/privacy}","received_events_url":"https://api.github.com/users/xinzweb/received_events","type":"User","site_admin":false},"body":"Fixes double free issue when we use two different types (i.e. Stream and Hash) of bitmap indexes under an AND/OR Bitmap operator.\r\n\r\nConsider a query like `select count(1) from bmcrash where bitmap_col = '999' AND btree_col1 = 'abcdefg999';` \r\nwhere we have a Btree index on `btree_col1` and a `BitmapIndex` on `bitmap_col`.\r\n\r\n```\r\n                                              QUERY PLAN\r\n------------------------------------------------------------------------------------------------------\r\n Aggregate  (cost=0.00..0.01 rows=1 width=8)\r\n   ->  Gather Motion 3:1  (slice1; segments: 3)  (cost=0.00..0.01 rows=1 width=8)\r\n         ->  Aggregate  (cost=0.00..0.01 rows=1 width=8)\r\n               ->  Bitmap Table Scan on bmcrash  (cost=0.00..0.01 rows=1 width=1)\r\n                     Recheck Cond: bitmap_col = '999'::text AND btree_col1::text = 'abcdefg999'::text\r\n                     ->  BitmapAnd  (cost=0.00..0.00 rows=0 width=0)\r\n                           ->  Bitmap Index Scan on bm_idx  (cost=0.00..0.00 rows=0 width=0)\r\n                                 Index Cond: bitmap_col = '999'::text\r\n                           ->  Bitmap Index Scan on bt_idx  (cost=0.00..0.00 rows=0 width=0)\r\n                                 Index Cond: btree_col1::text = 'abcdefg999'::text\r\n Settings:  optimizer=on\r\n Optimizer status: PQO version 1.665\r\n```\r\n\r\nIn the plan above, we have two `BitmapIndexScan` and a `BitmapAnd` operator that merges the two bitmaps, viz. a `StreamBitmap` (`bm_idx` on `bitmap_col`) \r\nand a `HashBitmap` (`bt_idx` on `btree_col1`) created at runtime from Btree.\r\n\r\nWhen we have mixed type of bitmaps connected using AND or OR, we wrap the `HashBitmap` into a `StreamBitmap` and combine them in a `OpStreamBitmap` (e.g. `BitmapAnd`), \r\nwhich contains pointers to the original two bitmaps. With this scheme there are two pointers to the same `HashBitmap` \r\n(one from the `OpStreamBitmap` and another in the `BitmapIndexScan` of `bt_idx`).\r\n\r\nSo when it comes time to free the `HashBitmap`, it gets freed twice - once using `tbm_stream_free` directly from `ExecEndBitmapIndexScan`, \r\nand then once again via `opstream_free` when we free the `OpStreamBitmap`\r\n\r\nExecEndBitmapIndexScan -> tbm_bitmap_free -> tbm_free\r\nExecEndBitmapIndexScan -> tbm_bitmap_free -> **opstream_free** -> tbm_stream_free -> tbm_free\r\n\r\nTo recap, we try to free only once per executor operator. But, the `tidbitmap.c` API `tbm_bitmap_free` will call `tbm_free` on the `HashBitmap` first \r\nand subsequently for second `OpStreamBitmap` (as produced by appending `HashBitmap` to the StreamBitmap) will iterate through all its contained bitmaps \r\nand call `tbm_free` on the `HashBitmap` again. This will result in a double free.\r\n\r\nTo fix this we keep track of the ownership of the underlying `HashBitmap` which is kept in `HashBitmapOpaque`, and only free the `HashBitmap` if the caller owns it.\r\n\r\n","created_at":"2016-08-30T23:31:33Z","updated_at":"2016-09-02T17:40:44Z","closed_at":null,"merged_at":null,"merge_commit_sha":"273382c600d52c429839fe78df01eeb7cbef4c9c","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089/commits","review_comments_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089/comments","review_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1089/comments","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/2addd8803bd046963085970b2b7277a6c22fcb05","head":{"label":"xinzweb:129402309_bitmap","ref":"129402309_bitmap","sha":"2addd8803bd046963085970b2b7277a6c22fcb05","user":{"login":"xinzweb","id":723898,"avatar_url":"https://avatars.githubusercontent.com/u/723898?v=3","gravatar_id":"","url":"https://api.github.com/users/xinzweb","html_url":"https://github.com/xinzweb","followers_url":"https://api.github.com/users/xinzweb/followers","following_url":"https://api.github.com/users/xinzweb/following{/other_user}","gists_url":"https://api.github.com/users/xinzweb/gists{/gist_id}","starred_url":"https://api.github.com/users/xinzweb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xinzweb/subscriptions","organizations_url":"https://api.github.com/users/xinzweb/orgs","repos_url":"https://api.github.com/users/xinzweb/repos","events_url":"https://api.github.com/users/xinzweb/events{/privacy}","received_events_url":"https://api.github.com/users/xinzweb/received_events","type":"User","site_admin":false},"repo":{"id":45497539,"name":"gpdb","full_name":"xinzweb/gpdb","owner":{"login":"xinzweb","id":723898,"avatar_url":"https://avatars.githubusercontent.com/u/723898?v=3","gravatar_id":"","url":"https://api.github.com/users/xinzweb","html_url":"https://github.com/xinzweb","followers_url":"https://api.github.com/users/xinzweb/followers","following_url":"https://api.github.com/users/xinzweb/following{/other_user}","gists_url":"https://api.github.com/users/xinzweb/gists{/gist_id}","starred_url":"https://api.github.com/users/xinzweb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xinzweb/subscriptions","organizations_url":"https://api.github.com/users/xinzweb/orgs","repos_url":"https://api.github.com/users/xinzweb/repos","events_url":"https://api.github.com/users/xinzweb/events{/privacy}","received_events_url":"https://api.github.com/users/xinzweb/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/xinzweb/gpdb","description":"Pivotal Greenplum Database","fork":true,"url":"https://api.github.com/repos/xinzweb/gpdb","forks_url":"https://api.github.com/repos/xinzweb/gpdb/forks","keys_url":"https://api.github.com/repos/xinzweb/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/xinzweb/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/xinzweb/gpdb/teams","hooks_url":"https://api.github.com/repos/xinzweb/gpdb/hooks","issue_events_url":"https://api.github.com/repos/xinzweb/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/xinzweb/gpdb/events","assignees_url":"https://api.github.com/repos/xinzweb/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/xinzweb/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/xinzweb/gpdb/tags","blobs_url":"https://api.github.com/repos/xinzweb/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/xinzweb/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/xinzweb/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/xinzweb/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/xinzweb/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/xinzweb/gpdb/languages","stargazers_url":"https://api.github.com/repos/xinzweb/gpdb/stargazers","contributors_url":"https://api.github.com/repos/xinzweb/gpdb/contributors","subscribers_url":"https://api.github.com/repos/xinzweb/gpdb/subscribers","subscription_url":"https://api.github.com/repos/xinzweb/gpdb/subscription","commits_url":"https://api.github.com/repos/xinzweb/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/xinzweb/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/xinzweb/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/xinzweb/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/xinzweb/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/xinzweb/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/xinzweb/gpdb/merges","archive_url":"https://api.github.com/repos/xinzweb/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/xinzweb/gpdb/downloads","issues_url":"https://api.github.com/repos/xinzweb/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/xinzweb/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/xinzweb/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/xinzweb/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/xinzweb/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/xinzweb/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/xinzweb/gpdb/deployments","created_at":"2015-11-03T21:41:51Z","updated_at":"2016-01-11T18:50:52Z","pushed_at":"2016-09-01T00:45:15Z","git_url":"git://github.com/xinzweb/gpdb.git","ssh_url":"git@github.com:xinzweb/gpdb.git","clone_url":"https://github.com/xinzweb/gpdb.git","svn_url":"https://github.com/xinzweb/gpdb","homepage":"http://greenplum.org","size":176502,"stargazers_count":0,"watchers_count":0,"language":"PLpgSQL","has_issues":false,"has_downloads":true,"has_wiki":false,"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":"greenplum-db:master","ref":"master","sha":"0cf6476d1b44036f488049c37a6ed12c6be0864e","user":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"repo":{"id":44781140,"name":"gpdb","full_name":"greenplum-db/gpdb","owner":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/greenplum-db/gpdb","description":"Greenplum Database","fork":false,"url":"https://api.github.com/repos/greenplum-db/gpdb","forks_url":"https://api.github.com/repos/greenplum-db/gpdb/forks","keys_url":"https://api.github.com/repos/greenplum-db/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/greenplum-db/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/greenplum-db/gpdb/teams","hooks_url":"https://api.github.com/repos/greenplum-db/gpdb/hooks","issue_events_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/greenplum-db/gpdb/events","assignees_url":"https://api.github.com/repos/greenplum-db/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/greenplum-db/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/greenplum-db/gpdb/tags","blobs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/greenplum-db/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/greenplum-db/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/greenplum-db/gpdb/languages","stargazers_url":"https://api.github.com/repos/greenplum-db/gpdb/stargazers","contributors_url":"https://api.github.com/repos/greenplum-db/gpdb/contributors","subscribers_url":"https://api.github.com/repos/greenplum-db/gpdb/subscribers","subscription_url":"https://api.github.com/repos/greenplum-db/gpdb/subscription","commits_url":"https://api.github.com/repos/greenplum-db/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/greenplum-db/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/greenplum-db/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/greenplum-db/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/greenplum-db/gpdb/merges","archive_url":"https://api.github.com/repos/greenplum-db/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/greenplum-db/gpdb/downloads","issues_url":"https://api.github.com/repos/greenplum-db/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/greenplum-db/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/greenplum-db/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/greenplum-db/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/greenplum-db/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/greenplum-db/gpdb/deployments","created_at":"2015-10-23T00:25:17Z","updated_at":"2016-09-02T16:45:50Z","pushed_at":"2016-09-02T17:40:00Z","git_url":"git://github.com/greenplum-db/gpdb.git","ssh_url":"git@github.com:greenplum-db/gpdb.git","clone_url":"https://github.com/greenplum-db/gpdb.git","svn_url":"https://github.com/greenplum-db/gpdb","homepage":"http://greenplum.org","size":227910,"stargazers_count":1591,"watchers_count":1591,"language":"PLpgSQL","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":441,"mirror_url":null,"open_issues_count":109,"forks":441,"open_issues":109,"watchers":1591,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/1089"},"issue":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/1089"},"comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/1089/comments"},"review_comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089/comments"},"review_comment":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1089/commits"},"statuses":{"href":"https://api.github.com/repos/greenplum-db/gpdb/statuses/2addd8803bd046963085970b2b7277a6c22fcb05"}}}}
{"actor":{"display_login":"karthijrk"}}