kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [6/6] kudu git commit: java: fix minor synchronization issues exposed by error-prone
Date Mon, 30 Apr 2018 21:10:21 GMT
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 <granthenke@apache.org>
Tested-by: Todd Lipcon <todd@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 <todd@apache.org>
Authored: Wed Apr 25 11:47:45 2018 -0700
Committer: Todd Lipcon <todd@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;
       }


Mime
View raw message