[jira] [Resolved] (KUDU-1700) Debug build will not fail gracefully on Messenger::Init() failure

["Sailesh Mukil (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/KUDU-1700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sailesh Mukil resolved KUDU-1700.
---------------------------------
       Resolution: Fixed
    Fix Version/s: 1.1.0

Commit in: https://github.com/apache/kudu/commit/d065e3316eb631e98f8c5da0b5223392da7e719e

> Debug build will not fail gracefully on Messenger::Init() failure
> -----------------------------------------------------------------
>
>                 Key: KUDU-1700
>                 URL: https://issues.apache.org/jira/browse/KUDU-1700
>             Project: Kudu
>          Issue Type: Bug
>          Components: rpc
>    Affects Versions: 0.10.0
>            Reporter: Sailesh Mukil
>            Priority: Minor
>              Labels: easyfix
>             Fix For: 1.1.0
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> In messenger.cc:
> {code}
> Status MessengerBuilder::Build(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();
> }
> {code}
> If Messenger::Init() fails, _msgr_ will not get assigned. Therefore, the calling function will also not have the argument _msgr_ assigned:
> {code}
> Status MessengerBuilder::Build(shared_ptr<Messenger> *msgr) {
>   Messenger *ptr;
>   RETURN_NOT_OK(Build(&ptr));
>   // See docs on Messenger::retain_self_ for info about this odd hack.
>   *msgr = shared_ptr<Messenger>(
>     ptr, std::mem_fun(&Messenger::AllExternalReferencesDropped));
>   return Status::OK();
> }
> {code}
> This will cause the deleter to not get assigned as well, which means Messenger::Shutdown() doesn't get called, which will ultimately result in a CHECK() here, as _closing\_ _ will not be set:
> {code}
> Messenger::~Messenger() {
>   std::lock_guard<percpu_rwlock> guard(lock_);
>   CHECK(closing_) << "Should have already shut down";
>   STLDeleteElements(&reactors_);
> }
> {code}
> Reordering the code in both the functions slightly will ensure that this does not happen.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)