You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2018/04/30 21:10:21 UTC
[6/6] kudu git commit: java: fix minor synchronization issues exposed
by error-prone
java: fix minor synchronization issues exposed by error-prone
* Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions().
This isn't likely to cause problems since it only happens on 'close'
and would only race if a new session was concurrently started.
* Add missing GuardedBy annotations in a few spots (non-functional)
* Fix synchronization in TableLocationsCache.toString() to properly
acquire the lock. Unlikely to cause problems in practice since we only
call toString() on this in some rare trace messages (if that).
* Fix synchronization in FakeDNS (test-only code)
Change-Id: I1c737f59928f393883d198872419e8832dfff006
Reviewed-on: http://gerrit.cloudera.org:8080/10199
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/69fb524c
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/69fb524c
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/69fb524c
Branch: refs/heads/master
Commit: 69fb524c413ee39f87e8a908171274ba9727e194
Parents: eac9d17
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Apr 25 11:47:45 2018 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Apr 30 20:56:51 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/kudu/client/AsyncKuduClient.java | 2 +-
.../src/main/java/org/apache/kudu/client/Connection.java | 1 +
.../java/org/apache/kudu/client/TableLocationsCache.java | 7 ++++++-
.../src/test/java/org/apache/kudu/client/FakeDNS.java | 11 ++++++++---
4 files changed, 16 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/69fb524c/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index 383df92..83d0684 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -2090,7 +2090,7 @@ public class AsyncKuduClient implements AutoCloseable {
synchronized (sessions) {
copyOfSessions = new HashSet<>(sessions);
}
- if (sessions.isEmpty()) {
+ if (copyOfSessions.isEmpty()) {
return Deferred.fromResult(null);
}
// Guaranteed that we'll have at least one session to close.
http://git-wip-us.apache.org/repos/asf/kudu/blob/69fb524c/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index 5401000..5d3155b 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -722,6 +722,7 @@ class Connection extends SimpleChannelUpstreamHandler {
}
/** Initiate opening TCP connection to the server. */
+ @GuardedBy("lock")
private void connect() {
Preconditions.checkState(lock.isHeldByCurrentThread());
Preconditions.checkState(state == State.NEW);
http://git-wip-us.apache.org/repos/asf/kudu/blob/69fb524c/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
index d8e5941..03d80dc 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
@@ -234,7 +234,12 @@ class TableLocationsCache {
@Override
public String toString() {
- return entries.values().toString();
+ rwl.readLock().lock();
+ try {
+ return entries.values().toString();
+ } finally {
+ rwl.readLock().unlock();
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/kudu/blob/69fb524c/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java b/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
index 25ff5ed..db9cd04 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
@@ -91,8 +91,10 @@ public class FakeDNS {
@Override
public InetAddress[] lookupAllHostAddr(String host)
throws UnknownHostException {
-
- InetAddress inetAddress = forwardResolutions.get(host);
+ InetAddress inetAddress;
+ synchronized(FakeDNS.this) {
+ inetAddress = forwardResolutions.get(host);
+ }
if (inetAddress != null) {
return new InetAddress[]{inetAddress};
}
@@ -107,7 +109,10 @@ public class FakeDNS {
return InetAddresses.toAddrString(InetAddress.getByAddress(addr));
}
- String hostname = reverseResolutions.get(InetAddress.getByAddress(addr));
+ String hostname;
+ synchronized (FakeDNS.this) {
+ hostname = reverseResolutions.get(InetAddress.getByAddress(addr));
+ }
if (hostname != null) {
return hostname;
}