kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject kudu git commit: KUDU-1993: fixed validation of 'grouped' gflags
Date Fri, 05 May 2017 20:49:18 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.3.x 49c9d6eb8 -> 1aa34a1d2


KUDU-1993: fixed validation of 'grouped' gflags

Added generic implementation for grouped gflags validators.

Use CUSTOM_FLAG_VALIDATOR() macro to register late-phase validator
function for command-line flags. The validation is performed upon call
of HandleCommonFlags() invoked by the top-level ParseCommandLineFlags()
function.

Added unit test to cover the new functionality.

Updated validators for security-related RPC and embedded webserver
flags.

As a workaround for LSAN reports on the leaks in case of ASAN build,
added two scoped leak check disablers into CreateAndStartTimeoutThread.
That does not seem harmful or hiding any potential leaks since it
affects only the timeout thread itself.  Opened separate JIRA to track
the (false positive?) leak warning: KUDU-1995.

Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Reviewed-on: http://gerrit.cloudera.org:8080/6795
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3)
Reviewed-on: http://gerrit.cloudera.org:8080/6806
Reviewed-by: Jean-Daniel Cryans <jdcryans@apache.org>


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

Branch: refs/heads/branch-1.3.x
Commit: 1aa34a1d20a93e7e1eb0ef659edee20a66423c23
Parents: 49c9d6e
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Wed May 3 22:33:22 2017 -0700
Committer: Jean-Daniel Cryans <jdcryans@apache.org>
Committed: Fri May 5 15:13:38 2017 +0000

----------------------------------------------------------------------
 src/kudu/rpc/messenger.cc             |  56 +++++-----
 src/kudu/server/webserver_options.cc  |  19 ++--
 src/kudu/util/CMakeLists.txt          |   2 +
 src/kudu/util/flag_validators-test.cc | 172 +++++++++++++++++++++++++++++
 src/kudu/util/flag_validators.cc      |  67 +++++++++++
 src/kudu/util/flag_validators.h       | 102 +++++++++++++++++
 src/kudu/util/flags.cc                |  17 +++
 src/kudu/util/test_main.cc            |  14 ++-
 8 files changed, 414 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/rpc/messenger.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 9f9574a..f13adb2 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -22,14 +22,15 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include <boost/algorithm/string/predicate.hpp>
-#include <gflags/gflags.h>
-#include <glog/logging.h>
 #include <list>
 #include <mutex>
 #include <set>
 #include <string>
 
+#include <boost/algorithm/string/predicate.hpp>
+#include <gflags/gflags.h>
+#include <glog/logging.h>
+
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stl_util.h"
@@ -48,6 +49,7 @@
 #include "kudu/security/token_verifier.h"
 #include "kudu/util/errno.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/socket.h"
@@ -127,56 +129,60 @@ static Status ParseTriState(const char* flag_name, const string&
flag_value, T*
   return Status::OK();
 }
 
-static bool ValidateRpcAuthentication(const char* /*flag_name*/, const string& /*flag_value*/)
{
-  RpcAuthentication authentication;
-  Status s = ParseTriState("--rpc_authentication", FLAGS_rpc_authentication, &authentication);
+static bool ValidateRpcAuthentication(const char* flag_name, const string& flag_value)
{
+  RpcAuthentication result;
+  Status s = ParseTriState(flag_name, flag_value, &result);
   if (!s.ok()) {
     LOG(ERROR) << s.message().ToString();
     return false;
   }
+  return true;
+}
+DEFINE_validator(rpc_authentication, &ValidateRpcAuthentication);
 
-  RpcEncryption encryption;
-  s = ParseTriState("--rpc_encryption", FLAGS_rpc_encryption, &encryption);
+static bool ValidateRpcEncryption(const char* flag_name, const string& flag_value) {
+  RpcEncryption result;
+  Status s = ParseTriState(flag_name, flag_value, &result);
   if (!s.ok()) {
-    // This will be caught by the rpc_encryption validator.
-    return true;
-  }
-
-  if (encryption == RpcEncryption::DISABLED && authentication != RpcAuthentication::DISABLED)
{
-    LOG(ERROR) << "RPC authentication (--rpc_authentication) must be disabled "
-                  "if RPC encryption (--rpc_encryption) is disabled";
+    LOG(ERROR) << s.message().ToString();
     return false;
   }
-
   return true;
 }
-DEFINE_validator(rpc_authentication, &ValidateRpcAuthentication);
+DEFINE_validator(rpc_encryption, &ValidateRpcEncryption);
+
+static bool ValidateRpcAuthnFlags() {
+  RpcAuthentication authentication;
+  CHECK_OK(ParseTriState("--rpc_authentication", FLAGS_rpc_authentication, &authentication));
 
-static bool ValidateRpcEncryption(const char* /*flag_name*/, const string& /*flag_value*/)
{
   RpcEncryption encryption;
-  Status s = ParseTriState("--rpc_encryption", FLAGS_rpc_encryption, &encryption);
-  if (!s.ok()) {
-    LOG(ERROR) << s.message().ToString();
+  CHECK_OK(ParseTriState("--rpc_encryption", FLAGS_rpc_encryption, &encryption));
+
+  if (encryption == RpcEncryption::DISABLED && authentication != RpcAuthentication::DISABLED)
{
+    LOG(ERROR) << "RPC authentication (--rpc_authentication) must be disabled "
+                  "if RPC encryption (--rpc_encryption) is disabled";
     return false;
   }
+
   return true;
 }
-DEFINE_validator(rpc_encryption, &ValidateRpcEncryption);
+GROUP_FLAG_VALIDATOR(rpc_authn_flags, ValidateRpcAuthnFlags);
 
-static bool ValidatePkiFlags(const char* /*flag_name*/, const string& /*flag_value*/)
{
+static bool ValidatePkiFlags() {
   bool has_cert = !FLAGS_rpc_certificate_file.empty();
   bool has_key = !FLAGS_rpc_private_key_file.empty();
   bool has_ca = !FLAGS_rpc_ca_certificate_file.empty();
 
   if (has_cert != has_key || has_cert != has_ca) {
     LOG(ERROR) << "--rpc_certificate_file, --rpc_private_key_file, and "
-                  "--rpc_ca_certificate_file flags must be set as a group";
+                  "--rpc_ca_certificate_file flags must be set as a group; "
+                  "i.e. either set all or none of them.";
     return false;
   }
 
   return true;
 }
-DEFINE_validator(rpc_certificate_file, &ValidatePkiFlags);
+GROUP_FLAG_VALIDATOR(pki_flags, ValidatePkiFlags);
 
 MessengerBuilder::MessengerBuilder(std::string name)
     : name_(std::move(name)),

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/server/webserver_options.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver_options.cc b/src/kudu/server/webserver_options.cc
index 2b5a6e0..860ecd3 100644
--- a/src/kudu/server/webserver_options.cc
+++ b/src/kudu/server/webserver_options.cc
@@ -17,13 +17,15 @@
 
 #include "kudu/server/webserver_options.h"
 
+#include <cstring>
+#include <cstdlib>
 #include <string>
+
 #include <gflags/gflags.h>
-#include <string.h>
-#include <stdlib.h>
 
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
 
 using std::string;
 
@@ -54,10 +56,11 @@ TAG_FLAG(webserver_enable_doc_root, advanced);
 // SSL configuration.
 DEFINE_string(webserver_certificate_file, "",
     "The location of the debug webserver's SSL certificate file, in PEM format. If "
-    "empty, webserver SSL support is not enabled");
+    "empty, webserver SSL support is not enabled. If --webserver_private_key_file "
+    "is set, this option must be set as well.");
 DEFINE_string(webserver_private_key_file, "", "The full path to the private key used as a"
-    " counterpart to the public key contained in --ssl_server_certificate. If "
-    "--ssl_server_certificate is set, this option must be set as well.");
+    " counterpart to the public key contained in --webserver_certificate_file. If "
+    "--webserver_certificate_file is set, this option must be set as well.");
 DEFINE_string(webserver_private_key_password_cmd, "", "A Unix command whose output "
     "returns the password used to decrypt the Webserver's certificate private key file "
     "specified in --webserver_private_key_file. If the PEM key file is not "
@@ -84,14 +87,14 @@ TAG_FLAG(webserver_port, stable);
 
 namespace kudu {
 
-static bool ValidateTlsFlags(const char* /*flag_name*/, const string& /*flag_value*/)
{
+static bool ValidateTlsFlags() {
   bool has_cert = !FLAGS_webserver_certificate_file.empty();
   bool has_key = !FLAGS_webserver_private_key_file.empty();
   bool has_passwd = !FLAGS_webserver_private_key_password_cmd.empty();
 
   if (has_key != has_cert) {
     LOG(ERROR) << "--webserver_certificate_file and --webserver_private_key_file "
-                  "must be set as a group";
+                  "must be set as a group; i.e. either set all or none of them.";
     return false;
   }
   if (has_passwd && !has_key) {
@@ -102,7 +105,7 @@ static bool ValidateTlsFlags(const char* /*flag_name*/, const string&
/*flag_val
 
   return true;
 }
-DEFINE_validator(webserver_private_key_file, &ValidateTlsFlags);
+GROUP_FLAG_VALIDATOR(webserver_tls_options, ValidateTlsFlags);
 
 // Returns KUDU_HOME if set, otherwise we won't serve any static files.
 static string GetDefaultDocumentRoot() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt
index 7d049a9..74ee741 100644
--- a/src/kudu/util/CMakeLists.txt
+++ b/src/kudu/util/CMakeLists.txt
@@ -138,6 +138,7 @@ set(UTIL_SRCS
   file_cache.cc
   flags.cc
   flag_tags.cc
+  flag_validators.cc
   group_varint.cc
   pstack_watcher.cc
   hdr_histogram.cc
@@ -340,6 +341,7 @@ ADD_KUDU_TEST(failure_detector-test)
 ADD_KUDU_TEST(file_cache-test)
 ADD_KUDU_TEST(file_cache-stress-test RUN_SERIAL true)
 ADD_KUDU_TEST(flag_tags-test)
+ADD_KUDU_TEST(flag_validators-test)
 ADD_KUDU_TEST(flags-test)
 ADD_KUDU_TEST(group_varint-test)
 ADD_KUDU_TEST(hash_util-test)

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/flag_validators-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flag_validators-test.cc b/src/kudu/util/flag_validators-test.cc
new file mode 100644
index 0000000..0bf553f
--- /dev/null
+++ b/src/kudu/util/flag_validators-test.cc
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include <gflags/gflags.h>
+#include <gtest/gtest.h>
+
+#include "kudu/util/debug/leakcheck_disabler.h"
+#include "kudu/util/flags.h"
+#include "kudu/util/flag_validators.h"
+#include "kudu/util/logging.h"
+#include "kudu/util/test_util.h"
+
+DEFINE_string(grouped_0, "", "First flag to set.");
+DEFINE_string(grouped_1, "", "Second flag to set.");
+
+namespace kudu {
+
+static bool CheckGroupedFlags() {
+  const bool is_set_0 = !FLAGS_grouped_0.empty();
+  const bool is_set_1 = !FLAGS_grouped_1.empty();
+
+  if (is_set_0 != is_set_1) {
+    LOG(ERROR) << "--grouped_0 and --grouped_1 must be set as a group";
+    return false;
+  }
+
+  return true;
+}
+GROUP_FLAG_VALIDATOR(test_group_validator, CheckGroupedFlags)
+
+class FlagsValidatorsBasicTest : public KuduTest {
+ public:
+  void RunTest(const char** argv, int argc) {
+    char** casted_argv = const_cast<char**>(argv);
+    // ParseCommandLineFlags() calls exit(1) if it finds inconsistency in flags.
+    ASSERT_EQ(1, ParseCommandLineFlags(&argc, &casted_argv, true));
+  }
+};
+
+TEST_F(FlagsValidatorsBasicTest, Grouped) {
+  const auto& validators = GetFlagValidators();
+  ASSERT_EQ(1, validators.size());
+  const auto& validator = validators.begin()->second;
+  EXPECT_TRUE(validator());
+  FLAGS_grouped_0 = "0";
+  EXPECT_FALSE(validator());
+  FLAGS_grouped_1 = "1";
+  EXPECT_TRUE(validator());
+  FLAGS_grouped_0 = "";
+  EXPECT_FALSE(validator());
+  FLAGS_grouped_1 = "";
+  EXPECT_TRUE(validator());
+}
+
+class FlagsValidatorsDeathTest : public KuduTest {
+ public:
+  void Run(const char** argv, int argc) {
+    debug::ScopedLeakCheckDisabler disabler;
+    char** casted_argv = const_cast<char**>(argv);
+    // ParseCommandLineFlags() calls exit(1) if one of the custom validators
+    // finds inconsistency in flags.
+    ParseCommandLineFlags(&argc, &casted_argv, true);
+    exit(0);
+  }
+
+  void RunSuccess(const char** argv, int argc) {
+    EXPECT_EXIT(Run(argv, argc), ::testing::ExitedWithCode(0), ".*");
+  }
+
+  void RunFailure(const char** argv, int argc) {
+    EXPECT_EXIT(Run(argv, argc), ::testing::ExitedWithCode(1),
+        ".* Detected inconsistency in command-line flags; exiting");
+  }
+};
+
+TEST_F(FlagsValidatorsDeathTest, GroupedSuccessNoFlags) {
+  const char* argv[] = { "argv_set_0" };
+  NO_FATALS(RunSuccess(argv, ARRAYSIZE(argv)));
+}
+
+TEST_F(FlagsValidatorsDeathTest, GroupedSuccessSimple) {
+  static const size_t kArgvSize = 1 + 2;
+  const char* argv_sets[][kArgvSize] = {
+    {
+      "argv_set_0",
+      "--grouped_0=first",
+      "--grouped_1=second",
+    },
+    {
+      "argv_set_1",
+      "--grouped_0=second",
+      "--grouped_1=first",
+    },
+    {
+      "argv_set_2",
+      "--grouped_0=",
+      "--grouped_1=",
+    },
+    {
+      "argv_set_3",
+      "--grouped_1=",
+      "--grouped_0=",
+    },
+  };
+  for (auto argv : argv_sets) {
+    RunSuccess(argv, kArgvSize);
+  }
+}
+
+TEST_F(FlagsValidatorsDeathTest, GroupedFailureSimple) {
+  static const size_t kArgvSize = 1 + 1;
+  const char* argv_sets[][kArgvSize] = {
+    {
+      "argv_set_0",
+      "--grouped_0=a",
+    },
+    {
+      "argv_set_1",
+      "--grouped_1=b",
+    },
+  };
+  for (auto argv : argv_sets) {
+    RunFailure(argv, kArgvSize);
+  }
+}
+
+TEST_F(FlagsValidatorsDeathTest, GroupedFailureWithEmptyValues) {
+  static const size_t kArgvSize = 1 + 2;
+  const char* argv_sets[][kArgvSize] = {
+    {
+      "argv_set_0",
+      "--grouped_0=a",
+      "--grouped_1=",
+    },
+    {
+      "argv_set_1",
+      "--grouped_1=",
+      "--grouped_0=a",
+    },
+    {
+      "argv_set_2",
+      "--grouped_0=",
+      "--grouped_1=b",
+    },
+    {
+      "argv_set_3",
+      "--grouped_1=b",
+      "--grouped_0=",
+    },
+  };
+  for (auto argv : argv_sets) {
+    RunFailure(argv, kArgvSize);
+  }
+}
+
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/flag_validators.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flag_validators.cc b/src/kudu/util/flag_validators.cc
new file mode 100644
index 0000000..f90fe2e
--- /dev/null
+++ b/src/kudu/util/flag_validators.cc
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/util/flag_validators.h"
+
+#include <string>
+
+#include "kudu/gutil/map-util.h"
+#include "kudu/gutil/singleton.h"
+
+using std::string;
+
+namespace kudu {
+namespace flag_validation_internal {
+
+// A singleton registry for storing group flag validators.
+class FlagValidatorRegistry {
+ public:
+  static FlagValidatorRegistry* GetInstance() {
+    return Singleton<FlagValidatorRegistry>::get();
+  }
+
+  void Register(const string& name, const FlagValidator& func) {
+    InsertOrDie(&validators_, name, func);
+  }
+
+  const FlagValidatorsMap& validators() {
+    return validators_;
+  }
+
+ private:
+  friend class Singleton<FlagValidatorRegistry>;
+  FlagValidatorRegistry() {}
+
+  FlagValidatorsMap validators_;
+
+  DISALLOW_COPY_AND_ASSIGN(FlagValidatorRegistry);
+};
+
+
+Registrator::Registrator(const char* name, const FlagValidator& validator) {
+  FlagValidatorRegistry::GetInstance()->Register(name, validator);
+}
+
+} // namespace flag_validation_internal
+
+
+const FlagValidatorsMap& GetFlagValidators() {
+  using flag_validation_internal::FlagValidatorRegistry;
+  return FlagValidatorRegistry::GetInstance()->validators();
+}
+
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/flag_validators.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/flag_validators.h b/src/kudu/util/flag_validators.h
new file mode 100644
index 0000000..02cc2dd
--- /dev/null
+++ b/src/kudu/util/flag_validators.h
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "kudu/gutil/macros.h"
+
+#include <functional>
+#include <map>
+#include <string>
+
+namespace kudu {
+
+// The validation function: takes no parameters and returns a boolean. A group
+// validator should return 'true' if validation was successful, or 'false'
+// otherwise.
+typedef std::function<bool(void)> FlagValidator;
+
+// The group validator registry's representation for as seen from the outside:
+// the key is the name of the group validator, the value is the validation
+// function.
+typedef std::map<std::string, FlagValidator> FlagValidatorsMap;
+
+// Register a 'group' validator for command-line flags. In contrast with the
+// standard (built-in) gflag validators registered by the DEFINE_validator()
+// macro, group validators are run at a later phase in the context of the main()
+// function. A group validator has a guarantee that all command-line flags have
+// been parsed, individually validated (via standard validators), and their
+// values are already set at the time when the validator runs.
+//
+// The first macro parameter is the name of the validator, the second parameter
+// is the validation function as is. The name must be unique across all
+// registered group validators.
+//
+// The validation function takes no parameters and returns 'true' in case of
+// successful validation, otherwise it returns 'false'. If at least one of the
+// registered group validators returns 'false', exit(1) is called.
+//
+// Usage guideline:
+//
+//   * Use the DEFINE_validator() macro if you need to validate an individual
+//     gflag's value
+//
+//   * Use the GROUP_FLAG_VALIDATOR() macro only if you need to validate a set
+//     of gflag values against one another, having the guarantee that their
+//     values are already set when the validation function runs.
+//
+// Sample usage:
+//
+//  static bool ValidateGroupedFlags() {
+//    bool has_a = !FLAGS_a.empty();
+//    bool has_b = !FLAGS_b.empty();
+//
+//    if (has_a != has_b) {
+//      LOG(ERROR) << "--a and --b must be set as a group";
+//      return false;
+//    }
+//
+//    return true;
+//  }
+//  GROUP_FLAG_VALIDATOR(grouped_flags_validator, ValidateGroupedFlags);
+//
+#define GROUP_FLAG_VALIDATOR(name, func) \
+  namespace {                                               \
+    ::kudu::flag_validation_internal::Registrator v_##name( \
+        AS_STRING(name), (func));                           \
+  }
+
+// Get all registered group flag validators.
+const FlagValidatorsMap& GetFlagValidators();
+
+namespace flag_validation_internal {
+
+// This is a utility class which registers a group validator upon instantiation.
+class Registrator {
+ public:
+  // The constructor registers a group validator with the specified name and
+  // the given validation function. The name must be unique among all group
+  // validators.
+  Registrator(const char* name, const FlagValidator& validator);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(Registrator);
+};
+
+} // namespace flag_validation_internal
+
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/flags.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index c410c19..ed9b34b 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -18,6 +18,7 @@
 #include "kudu/util/flags.h"
 
 #include <iostream>
+#include <map>
 #include <sstream>
 #include <string>
 #include <unordered_set>
@@ -33,6 +34,7 @@
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/os-util.h"
@@ -417,6 +419,20 @@ void CheckFlagsAllowed() {
   }
 }
 
+// Run 'late phase' custom validators: these can be run only when all flags are
+// already parsed and individually validated.
+void RunCustomValidators() {
+  const auto& validators(GetFlagValidators());
+  bool found_inconsistency = false;
+  for (const auto& e : validators) {
+    found_inconsistency = !e.second();
+  }
+  if (found_inconsistency) {
+    LOG(ERROR) << "Detected inconsistency in command-line flags; exiting";
+    exit(1);
+  }
+}
+
 // Redact the flag tagged as 'sensitive', if --redact is set
 // with 'flag'. Otherwise, return its value as-is. If EscapeMode
 // is set to HTML, return HTML escaped string.
@@ -457,6 +473,7 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags)
{
 
 void HandleCommonFlags() {
   CheckFlagsAllowed();
+  RunCustomValidators();
 
   if (FLAGS_helpxml) {
     DumpFlagsXML();

http://git-wip-us.apache.org/repos/asf/kudu/blob/1aa34a1d/src/kudu/util/test_main.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_main.cc b/src/kudu/util/test_main.cc
index aa64387..fa88f21 100644
--- a/src/kudu/util/test_main.cc
+++ b/src/kudu/util/test_main.cc
@@ -15,13 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <stdlib.h>
+#include <cstdlib>
+#include <thread>
 
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 #include <gtest/gtest.h>
-#include <thread>
 
+#include "kudu/util/debug/leakcheck_disabler.h"
 #include "kudu/util/pstack_watcher.h"
 #include "kudu/util/flags.h"
 #include "kudu/util/minidump.h"
@@ -42,7 +43,16 @@ namespace kudu {
 // the tests complete.
 static void CreateAndStartTimeoutThread() {
   if (FLAGS_test_timeout_after == 0) return;
+
+  // KUDU-1995: if running death tests using EXPECT_EXIT()/ASSERT_EXIT(), LSAN
+  // reports leaks in CreateAndStartTimeoutThread(). Adding a couple of scoped
+  // leak check disablers as a workaround since right now it's not clear what
+  // is going on exactly: LSAN does not report those leaks for tests which run
+  // ASSERT_DEATH(). This does not seem harmful or hiding any potential leaks
+  // since it's scoped and targeted only for this utility thread.
+  debug::ScopedLeakCheckDisabler disabler;
   std::thread([=](){
+      debug::ScopedLeakCheckDisabler disabler;
       SleepFor(MonoDelta::FromSeconds(FLAGS_test_timeout_after));
       // Dump a pstack to stdout.
       WARN_NOT_OK(PstackWatcher::DumpStacks(), "Unable to print pstack");


Mime
View raw message