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 <t...@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 <sail...@apache.com> Authored: Thu Oct 13 21:07:29 2016 -0700 Committer: Todd Lipcon <t...@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_;