You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/10/14 06:37:50 UTC
kudu git commit: KUDU-1700: Debug build will not fail gracefully on
Messenger::Init() failure
Repository: kudu
Updated Branches:
refs/heads/master 1746ae021 -> d065e3316
KUDU-1700: Debug build will not fail gracefully on Messenger::Init() failure
If Messenger::Init() fails in a debug build, a CHECK() will happen on
the destruction of the Messenger object. This happens beacuse, on an
error, the Messenger is not Shutdown().
This patch gets rid of the private function
Messenger::Build(Messenger**) and moves all its logic into the public
function Build(shared_ptr<Messenger>*). On an error, an explicit call
to AllExternalReferencesDropped() is made.
This case was probably never encountered as Messenger::Init()
currently has a very low chance of failing. However, in the future, as
more things may get added on to the function, this issue might show
up more often.
A more detailed explanation is given in the JIRA.
Change-Id: Id6021587c746af53305b3f601bb1bcc19f63eab0
Reviewed-on: http://gerrit.cloudera.org:8080/4724
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d065e331
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d065e331
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d065e331
Branch: refs/heads/master
Commit: d065e3316eb631e98f8c5da0b5223392da7e719e
Parents: 1746ae0
Author: Sailesh Mukil <sa...@apache.com>
Authored: Thu Oct 13 21:07:29 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Oct 14 06:37:31 2016 +0000
----------------------------------------------------------------------
src/kudu/rpc/messenger.cc | 17 +++++++----------
src/kudu/rpc/messenger.h | 1 -
2 files changed, 7 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/d065e331/src/kudu/rpc/messenger.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index b275f5b..a543f5d 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -100,21 +100,18 @@ MessengerBuilder &MessengerBuilder::set_metric_entity(
return *this;
}
-Status MessengerBuilder::Build(Messenger **msgr) {
+Status MessengerBuilder::Build(shared_ptr<Messenger> *msgr) {
RETURN_NOT_OK(SaslInit(kSaslAppName)); // Initialize SASL library before we start making requests
gscoped_ptr<Messenger> new_msgr(new Messenger(*this));
- RETURN_NOT_OK(new_msgr.get()->Init());
- *msgr = new_msgr.release();
- return Status::OK();
-}
-
-Status MessengerBuilder::Build(shared_ptr<Messenger> *msgr) {
- Messenger *ptr;
- RETURN_NOT_OK(Build(&ptr));
+ Status build_status = new_msgr->Init();
+ if (!build_status.ok()) {
+ new_msgr->AllExternalReferencesDropped();
+ return build_status;
+ }
// See docs on Messenger::retain_self_ for info about this odd hack.
*msgr = shared_ptr<Messenger>(
- ptr, std::mem_fun(&Messenger::AllExternalReferencesDropped));
+ new_msgr.release(), std::mem_fun(&Messenger::AllExternalReferencesDropped));
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/d065e331/src/kudu/rpc/messenger.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index bf0fc86..1424913 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -95,7 +95,6 @@ class MessengerBuilder {
Status Build(std::shared_ptr<Messenger> *msgr);
private:
- Status Build(Messenger **msgr);
const std::string name_;
MonoDelta connection_keepalive_time_;
int num_reactors_;