Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5120

to look at the new patch set (#3).

Change subject: KUDU-1749. Allow callers to disable SASL initialization
......................................................................

KUDU-1749. Allow callers to disable SASL initialization

The Kudu client makes use of SASL, as do other applications such as
Impala that use the client. This means that both Kudu and those
applications are expected to initialize SASL.

The sasl_*_init() functions themselves are safe to call multiple times
(they have no effect). However, sasl_set_mutex() is not safe to call
multiple times, nor is it safe to call after another library has already
begun using the SASL libraries.

IMPALA-4479 describes a crash in SaslMutexUnlock() which is likely
caused by this issue: Impala has initialized the SASL library with no
call to sasl_set_mutex(), and then during Kudu initialization we set the
mutex functions to our own. Then when we call sasl_dispose(), SASL tries
to acquire an internal mutex which was previously allocated to a garbage
value 0x1 by the built-in "fake" mutex implementation. This crashes when
we attempt to access it using our real mutex implementation.

The fix has a few parts:
- we have a new API which allows embedders to disable initialization of
  SASL.
- we try to auto-detect the case in which the client has fully
  initialized SASL including providing a mutex implementation. In that
  case, we skip initialization but log a warning that the caller should
  disable it explicitly.
- if we detect that it's initialized but no mutex implementation is
  provided, we'll log a warning on first usage of SASL.
- if they disable initialization but didn't initialize SASL on their
  own, we'll return an error on first usage.

The one sketchy thing about this patch is that the SASL API doesn't give
a 100% public way to determine its initialization state. However, it
does export a symbol which we can import that provides the necessary
information. This symbol is marked as an API, so we expect it to be
stable enough for usage, at least across the range of supported
operating systems.

Change-Id: I54d5ecfb2526852003f1ea249d1d2a8e6c0e91cd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
5 files changed, 183 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5120/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I54d5ecfb2526852003f1ea249d1d2a8e6c0e91cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>

Reply via email to