You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2017/01/05 20:36:57 UTC
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5615
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 9 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5615/1
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Marcel Kornacker, Impala Public Jenkins, Jim Apple, Henry Robinson,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5615
to look at the new patch set (#4).
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 10 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5615/4
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Reviewed-on: http://gerrit.cloudera.org:8080/5615
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 15 insertions(+), 17 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Dan Hecht: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 2: Verified-1
Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/149/
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 141
> AtomicInt::Add says it returns the new value. If the new value was greater
Right - if this wasn't the last decrement, then we can exit early because some other call will do the cleanup.
If if it's > 0 though we have no guarantee about when the cleanup will happen. If another thread is executing this function at the same time, it might clean up the QueryState immediately after this thread decrements it.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 6:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/160/
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Marcel Kornacker, Impala Public Jenkins, Jim Apple, Henry Robinson,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5615
to look at the new patch set (#3).
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 12 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5615/3
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 3: Verified-1
Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/150/
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 6:
I had to make a non-trivial change to fix a bug, would be good if reviewers could take another quick look.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 2:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/149/
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 4:
The previous version had a silly bug in "delete" qs, which was a NULL ptr, which meant that QueryStates were leaked.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 141
> AtomicInt::Add says it returns the new value. If the new value was greater
Some other thread could decrement the refcount to 0 and GC it.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 151: qs = it->second;
can you give this a new variable name to make it clear that it's not the same QueryState* as the argument? The comment makes it look like the usage on line 157 is a bug, even though it's actually safe.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 141
> Some other thread could decrement the refcount to 0 and GC it.
Yikes, of course.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 3:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/150/
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 141
AtomicInt::Add says it returns the new value. If the new value was greater than 0, wouldn't the refcount have been greater than 0, making qs still live?
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple, Henry Robinson,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5615
to look at the new patch set (#2).
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 12 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5615/2
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 151: qs = it->second;
> can you give this a new variable name to make it clear that it's not the sa
I ended up just using 'it' directly. We can also save a map looking by passing 'it' into erase below.
Line 155: // someone else might have increased the refcnt in the meantime
I won't touch this now but it doesn't seem like this should be possible.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 2:
Carry the +1
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Marcel Kornacker, Impala Public Jenkins, Jim Apple, Henry Robinson,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5615
to look at the new patch set (#6).
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
IMPALA-4707: fix use-after-free in QueryExecMgr
The bug is that the QueryState was referenced after the refcount is
decremented. The fix is to not do that.
Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
---
M be/src/runtime/query-exec-mgr.cc
1 file changed, 15 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5615/6
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 141
> Right - if this wasn't the last decrement, then we can exit early because s
Haha, missed your comments - never mind.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Carry +2
http://gerrit.cloudera.org:8080/#/c/5615/2/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 138: // under us
> revert changes to comment.
Done
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4707: fix use-after-free in QueryExecMgr
Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.
Change subject: IMPALA-4707: fix use-after-free in QueryExecMgr
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5615/2/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
Line 138: // under us.
revert changes to comment.
--
To view, visit http://gerrit.cloudera.org:8080/5615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I329ff758a0de148a3bdfedc245e56c3a63255535
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes