You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/06/30 21:37:52 UTC
[1/2] kudu git commit: Work around another OpenSSL thread safety bug
Repository: kudu
Updated Branches:
refs/heads/branch-1.4.x 91b7b5395 -> d759f3ee9
Work around another OpenSSL thread safety bug
In the course of debugging some CHECK failures and TSAN errors, I found
that older versions of OpenSSL have non-threadsafe OBJ_create and even
ERR_peek_error methods. This commit fixes an instance where we were
calling OBJ_create concurrently by wrapping it in a std::call_once. I
don't have a fix for ERR_peek_err unsafety, since that's used
pervasively in most methods touching OpenSSL.
Side note: for debugging issues like this, I find it helpful to run ASAN
with the following options:
ASAN_OPTIONS="fast_unwind_on_malloc=0"
That option typically makes races more reproducible, and produces better
stack traces as well.
Change-Id: I9a9fe1a32f77bf24a5c7e692a55b8ad96488d409
Reviewed-on: http://gerrit.cloudera.org:8080/6997
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
(cherry picked from commit d0270172e91bf6bcb045cb63a731609472fa6be4)
Reviewed-on: http://gerrit.cloudera.org:8080/7344
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1a897cca
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1a897cca
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1a897cca
Branch: refs/heads/branch-1.4.x
Commit: 1a897cca7d864ac8aa7e50455cecbdb02979a580
Parents: 91b7b53
Author: Dan Burkert <da...@apache.org>
Authored: Thu May 25 16:24:17 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 30 19:43:59 2017 +0000
----------------------------------------------------------------------
src/kudu/security/cert-test.cc | 24 ++++++++++++++++++++++++
src/kudu/security/cert.cc | 13 +++++++------
2 files changed, 31 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/1a897cca/src/kudu/security/cert-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert-test.cc b/src/kudu/security/cert-test.cc
index 275a60b..acd0f74 100644
--- a/src/kudu/security/cert-test.cc
+++ b/src/kudu/security/cert-test.cc
@@ -15,7 +15,9 @@
// specific language governing permissions and limitations
// under the License.
+#include <thread>
#include <utility>
+#include <vector>
#include <boost/optional.hpp>
#include <boost/optional/optional_io.hpp>
@@ -25,11 +27,14 @@
#include "kudu/security/crypto.h"
#include "kudu/security/openssl_util.h"
#include "kudu/security/test/test_certs.h"
+#include "kudu/util/barrier.h"
#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
using std::pair;
+using std::thread;
+using std::vector;
namespace kudu {
namespace security {
@@ -60,6 +65,25 @@ class CertTest : public KuduTest {
PrivateKey ca_exp_private_key_;
};
+// Regression test to make sure that GetKuduKerberosPrincipalOidNid is thread
+// safe. OpenSSL 1.0.0's OBJ_create method is not thread safe.
+TEST_F(CertTest, GetKuduKerberosPrincipalOidNidConcurrent) {
+ int kConcurrency = 16;
+ Barrier barrier(kConcurrency);
+
+ vector<thread> threads;
+ for (int i = 0; i < kConcurrency; i++) {
+ threads.emplace_back([&] () {
+ barrier.Wait();
+ CHECK_NE(NID_undef, GetKuduKerberosPrincipalOidNid());
+ });
+ }
+
+ for (auto& thread : threads) {
+ thread.join();
+ }
+}
+
// Check input/output of the X509 certificates in PEM format.
TEST_F(CertTest, CertInputOutputPEM) {
const Cert& cert = ca_cert_;
http://git-wip-us.apache.org/repos/asf/kudu/blob/1a897cca/src/kudu/security/cert.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index cde4e47..fa4c753 100644
--- a/src/kudu/security/cert.cc
+++ b/src/kudu/security/cert.cc
@@ -17,6 +17,7 @@
#include "kudu/security/cert.h"
+#include <mutex>
#include <string>
#include <openssl/evp.h>
@@ -57,12 +58,12 @@ string X509NameToString(X509_NAME* name) {
int GetKuduKerberosPrincipalOidNid() {
InitializeOpenSSL();
- SCOPED_OPENSSL_NO_PENDING_ERRORS;
-
- int nid = OBJ_txt2nid(kKuduKerberosPrincipalOidStr);
- if (nid != NID_undef) return nid;
- nid = OBJ_create(kKuduKerberosPrincipalOidStr, "kuduPrinc", "kuduKerberosPrincipal");
- CHECK_NE(nid, NID_undef) << "could not create kuduPrinc oid";
+ static std::once_flag flag;
+ static int nid;
+ std::call_once(flag, [&] () {
+ nid = OBJ_create(kKuduKerberosPrincipalOidStr, "kuduPrinc", "kuduKerberosPrincipal");
+ CHECK_NE(nid, NID_undef) << "failed to create kuduPrinc oid: " << GetOpenSSLErrors();
+ });
return nid;
}
[2/2] kudu git commit: java: fix incorrect hashmap usage in Statistics
Posted by ad...@apache.org.
java: fix incorrect hashmap usage in Statistics
65cb2edf5661599f689e28f4eec161fe062f7cb5 changed the hash map in the
Statistics class to use Strings instead of Slices as keys, but didn't
properly update all of the call sites.
Change-Id: I715e96a02ec83199b7e0678281c0857bf9258cf4
Reviewed-on: http://gerrit.cloudera.org:8080/7335
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit 332ebcd063b55923806b1704113d0623f7aae745)
Reviewed-on: http://gerrit.cloudera.org:8080/7338
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d759f3ee
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d759f3ee
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d759f3ee
Branch: refs/heads/branch-1.4.x
Commit: d759f3ee93d1158ac17409a1abc565b3e0d9b3d3
Parents: 1a897cc
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Jun 29 14:21:15 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 30 19:44:08 2017 +0000
----------------------------------------------------------------------
.../main/java/org/apache/kudu/client/Statistics.java | 10 ++--------
.../java/org/apache/kudu/client/TestStatistics.java | 13 +++++++++++--
2 files changed, 13 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/d759f3ee/java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
index 5f595c8..1c672c6 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
@@ -17,7 +17,6 @@
package org.apache.kudu.client;
-import java.nio.charset.Charset;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLongArray;
@@ -26,9 +25,6 @@ import com.google.common.collect.Sets;
import org.apache.kudu.annotations.InterfaceAudience;
import org.apache.kudu.annotations.InterfaceStability;
-import org.apache.kudu.util.Slice;
-import org.apache.kudu.util.Slices;
-
/**
* A Statistics belongs to a specific AsyncKuduClient. It stores client-level
@@ -101,8 +97,7 @@ public class Statistics {
* @return the value of the statistic
*/
public long getTabletStatistic(String tabletId, Statistic statistic) {
- Slice tabletIdAsSlice = Slices.copiedBuffer(tabletId, Charset.defaultCharset());
- TabletStatistics tabletStatistics = stsMap.get(tabletIdAsSlice);
+ TabletStatistics tabletStatistics = stsMap.get(tabletId);
if (tabletStatistics == null) {
return 0;
} else {
@@ -173,8 +168,7 @@ public class Statistics {
* @return table name
*/
public String getTableName(String tabletId) {
- Slice tabletIdAsSlice = Slices.copiedBuffer(tabletId, Charset.defaultCharset());
- TabletStatistics tabletStatistics = stsMap.get(tabletIdAsSlice);
+ TabletStatistics tabletStatistics = stsMap.get(tabletId);
if (tabletStatistics == null) {
return null;
} else {
http://git-wip-us.apache.org/repos/asf/kudu/blob/d759f3ee/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatistics.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatistics.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatistics.java
index fadbfde..7afb524 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatistics.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatistics.java
@@ -23,6 +23,9 @@ import org.junit.Test;
import org.apache.kudu.client.Statistics.Statistic;
+import java.util.ArrayList;
+import java.util.List;
+
public class TestStatistics extends BaseKuduTest {
private static final String TABLE_NAME = TestStatistics.class.getName() + "-"
@@ -69,7 +72,13 @@ public class TestStatistics extends BaseKuduTest {
assertEquals(rowCount, statistics.getClientStatistic(Statistic.OPS_ERRORS));
assertEquals(byteSize * 2, statistics.getClientStatistic(Statistic.BYTES_WRITTEN));
- assertEquals(1, statistics.getTableSet().size());
- assertEquals(1, statistics.getTabletSet().size());
+ List<String> tableNames = new ArrayList<>(statistics.getTableSet());
+ assertEquals(1, tableNames.size());
+ assertEquals(TABLE_NAME, tableNames.get(0));
+ assertEquals(rowCount, statistics.getTableStatistic(TABLE_NAME, Statistic.WRITE_OPS));
+
+ List<String> tabletIds = new ArrayList<>(statistics.getTabletSet());
+ assertEquals(1, tabletIds.size());
+ assertEquals(rowCount, statistics.getTabletStatistic(tabletIds.get(0), Statistic.WRITE_OPS));
}
}