kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject kudu git commit: Free SASL connection objects when negotiation completes
Date Fri, 21 Oct 2016 00:57:28 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 7138468a5 -> 1096d663f


Free SASL connection objects when negotiation completes

The SASL connections are only currently used during negotiation, and
then can be dropped following that. So, it makes sense to dispose of the
objects as soon as possible from a memory consumption standpoint,
since they probably hold some buffers.

More importantly, though, this also works around a bug I saw
occasionally when running a Kerberized Kudu CLI:

- the CLI main would make some RPC call and get an "unauthorized"
  status.
- it would then drop its reference to the KuduClient and exit.
- this would call Messenger::AllExternalReferencesDropped() which
  initiates an asynchronous shutdown of the reactor threads.
- the main thread would get to 'exit()' and start unloading dynamic
  libraries, including libkrb5.
- the reactor thread would call sasl_dispose on a sasl_connection_t
  during its shutdown sequence, which would crash with an assertion
  failure in k5_mutex.h if this happened after krb5 was unloaded.

Rather than futz with the shutdown sequence of the reactor, it was much
simpler to just dispose the connections earlier as done in this patch.

Change-Id: Ib7aada1e44a80af94c5c069e9f583aedcd78a68b
Reviewed-on: http://gerrit.cloudera.org:8080/4761
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1096d663
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1096d663
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1096d663

Branch: refs/heads/master
Commit: 1096d663f36da19b211dd4fdf76fca10f842b8b8
Parents: 7138468
Author: Todd Lipcon <todd@apache.org>
Authored: Wed Oct 19 11:02:31 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Thu Oct 20 22:02:24 2016 +0000

----------------------------------------------------------------------
 src/kudu/rpc/sasl_client.cc | 10 ++++++++++
 src/kudu/rpc/sasl_client.h  |  2 ++
 src/kudu/rpc/sasl_server.cc |  8 +++++++-
 src/kudu/rpc/sasl_server.h  |  2 ++
 4 files changed, 21 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1096d663/src/kudu/rpc/sasl_client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_client.cc b/src/kudu/rpc/sasl_client.cc
index 3ac47f8..cc252b2 100644
--- a/src/kudu/rpc/sasl_client.cc
+++ b/src/kudu/rpc/sasl_client.cc
@@ -38,6 +38,7 @@
 #include "kudu/util/faststring.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/net/socket.h"
+#include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/trace.h"
 
 namespace kudu {
@@ -171,6 +172,15 @@ Status SaslClient::Init(const string& service_type) {
 }
 
 Status SaslClient::Negotiate() {
+  // After negotiation, we no longer need the SASL library object, so
+  // may as well free its memory since the connection may be long-lived.
+  // Additionally, this works around a SEGV seen at process shutdown time:
+  // if we still have SASL objects retained by Reactor when the process
+  // is exiting, the SASL libraries may start destructing global state
+  // and cause a crash when we sasl_dispose the connection.
+  auto cleanup = MakeScopedCleanup([&]() {
+      sasl_conn_.reset();
+    });
   TRACE("Called SaslClient::Negotiate()");
 
   // Ensure we called exactly once, and in the right order.

http://git-wip-us.apache.org/repos/asf/kudu/blob/1096d663/src/kudu/rpc/sasl_client.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_client.h b/src/kudu/rpc/sasl_client.h
index a01865e..d98942a 100644
--- a/src/kudu/rpc/sasl_client.h
+++ b/src/kudu/rpc/sasl_client.h
@@ -146,6 +146,8 @@ class SaslClient {
   string app_name_;
   Socket sock_;
   std::vector<sasl_callback_t> callbacks_;
+  // The SASL connection object. This is initialized in Init() and
+  // freed after Negotiate() completes (regardless whether it was successful).
   gscoped_ptr<sasl_conn_t, SaslDeleter> sasl_conn_;
   SaslHelper helper_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/1096d663/src/kudu/rpc/sasl_server.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_server.cc b/src/kudu/rpc/sasl_server.cc
index 5cb5699..bdacc83 100644
--- a/src/kudu/rpc/sasl_server.cc
+++ b/src/kudu/rpc/sasl_server.cc
@@ -28,12 +28,13 @@
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stringprintf.h"
 #include "kudu/gutil/strings/split.h"
-#include "kudu/rpc/blocking_ops.h"
 #include "kudu/rpc/auth_store.h"
+#include "kudu/rpc/blocking_ops.h"
 #include "kudu/rpc/constants.h"
 #include "kudu/rpc/serialization.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/net/socket.h"
+#include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/trace.h"
 
 namespace kudu {
@@ -146,6 +147,11 @@ Status SaslServer::Init(const string& service_type) {
 }
 
 Status SaslServer::Negotiate() {
+  // After negotiation, we no longer need the SASL library object, so
+  // may as well free its memory since the connection may be long-lived.
+  auto cleanup = MakeScopedCleanup([&]() {
+      sasl_conn_.reset();
+    });
   DVLOG(4) << "Called SaslServer::Negotiate()";
 
   // Ensure we are called exactly once, and in the right order.

http://git-wip-us.apache.org/repos/asf/kudu/blob/1096d663/src/kudu/rpc/sasl_server.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_server.h b/src/kudu/rpc/sasl_server.h
index 2f9c194..0614139 100644
--- a/src/kudu/rpc/sasl_server.h
+++ b/src/kudu/rpc/sasl_server.h
@@ -144,6 +144,8 @@ class SaslServer {
   string app_name_;
   Socket sock_;
   std::vector<sasl_callback_t> callbacks_;
+  // The SASL connection object. This is initialized in Init() and
+  // freed after Negotiate() completes (regardless whether it was successful).
   gscoped_ptr<sasl_conn_t, SaslDeleter> sasl_conn_;
   SaslHelper helper_;
 


Mime
View raw message