Excavator: Upgrades Baseline to the latest version (#6470)

atlasdb-commons/src/main/java/com/palantir/common/concurrent/InterruptibleFuture.java

@@ -53,16 +53,16 @@ private enum State {
     private final Lock lock = new ReentrantLock(false);
     private final Condition condition = lock.newCondition();
 
-    @GuardedBy(value = "lock")
+    @GuardedBy("lock")
     private V returnValue;
 
-    @GuardedBy(value = "lock")
+    @GuardedBy("lock")
     private Throwable executionException;
 
-    @GuardedBy(value = "lock")
+    @GuardedBy("lock")
     private volatile CancellationException cancellationException;
 
-    @GuardedBy(value = "lock")
+    @GuardedBy("lock")
     private volatile State state = State.WAITING_TO_RUN;
 
     private final FutureTask<?> futureTask = new FutureTask<Void>(
@@ -113,7 +113,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
         return futureTask.cancel(mayInterruptIfRunning);
     }
 
-    @SuppressWarnings("GuardedByChecker")
+    @GuardedBy("lock")
     protected void noteFinished() {
         state = State.COMPLETED;
         condition.signalAll();
@@ -157,7 +157,7 @@ public final V get(long timeout, TimeUnit unit)
         }
     }
 
-    @SuppressWarnings("GuardedByChecker")
+    @GuardedBy("lock")
     private V getReturnValue() throws ExecutionException, CancellationException {
         if (cancellationException != null) {
             throw Throwables.chain(
@@ -173,15 +173,23 @@ private V getReturnValue() throws ExecutionException, CancellationException {
      * @return true if and only if the task was canceled before it ever executed
      */
     @Override
-    @SuppressWarnings("GuardedByChecker")
     public final boolean isCancelled() {
-        return cancellationException != null;
+        lock.lock();
+        try {
+            return cancellationException != null;
+        } finally {
+            lock.unlock();
+        }
     }
 
     @Override
-    @SuppressWarnings("GuardedByChecker")
     public final boolean isDone() {
-        return state == State.COMPLETED;
+        lock.lock();
+        try {
+            return state == State.COMPLETED;
+        } finally {
+            lock.unlock();
+        }
     }
 
     @Override

atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/timestamp/InDbTimestampBoundStore.java

@@ -62,8 +62,7 @@ protected String getInitializingClassName() {
     private final PhysicalBoundStoreStrategy physicalBoundStoreStrategy;
     private final InitializingWrapper wrapper = new InitializingWrapper();
 
-    @GuardedBy("this") // lazy init to avoid db connections in constructors
-    private DBType dbType;
+    private volatile DBType dbType;
 
     @GuardedBy("this")
     private Long currentLimit = null;
@@ -221,11 +220,17 @@ public synchronized void storeUpperLimit(final long limit) {
         });
     }
 
-    @GuardedBy("this")
     private DBType getDbType(Connection connection) {
-        if (dbType == null) {
-            dbType = ConnectionDbTypes.getDbType(connection);
+        DBType type = this.dbType; // volatile read
+        if (type == null) {
+            synchronized (this) {
+                type = this.dbType;
+                if (type == null) {
+                    type = ConnectionDbTypes.getDbType(connection);
+                    this.dbType = type;
+                }
+            }
         }
-        return dbType;
+        return type;
     }
 }

atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/keyvalue/api/cache/LockWatchValueScopingCacheImpl.java

@@ -198,15 +198,15 @@ private synchronized void processCommitUpdate(long startTimestamp) {
     /**
      * In order to maintain the necessary invariants, we need to do the following:
      * <p>
-     *  1. For each new event, we apply it to the cache. The effects of this application is described in
-     *     {@link LockWatchValueScopingCache}.
-     *  2. For each transaction, we must ensure that we store a snapshot of the cache at the sequence corresponding
-     *     to the transaction's start timestamp. Note that not every sequence will have a corresponding timestamp, so we
-     *     don't bother storing a snapshot for those sequences. Also note that we know that each call here will only
-     *     ever have new events, and that consecutive calls to this method will *always* have increasing sequences
-     *     (without this last guarantee, we'd need to store snapshots for all sequences).
-     *  3. For each transaction, we must create a transaction scoped cache. We do this now as we have tighter guarantees
-     *     around when the cache is created, and thus deleted.
+     * 1. For each new event, we apply it to the cache. The effects of this application is described in
+     * {@link LockWatchValueScopingCache}.
+     * 2. For each transaction, we must ensure that we store a snapshot of the cache at the sequence corresponding
+     * to the transaction's start timestamp. Note that not every sequence will have a corresponding timestamp, so we
+     * don't bother storing a snapshot for those sequences. Also note that we know that each call here will only
+     * ever have new events, and that consecutive calls to this method will *always* have increasing sequences
+     * (without this last guarantee, we'd need to store snapshots for all sequences).
+     * 3. For each transaction, we must create a transaction scoped cache. We do this now as we have tighter guarantees
+     * around when the cache is created, and thus deleted.
      */
     private synchronized void updateStores(TransactionsLockWatchUpdate updateForTransactions) {
         Multimap<Sequence, StartTimestamp> reversedMap = createSequenceTimestampMultimap(updateForTransactions);
@@ -246,7 +246,7 @@ private synchronized boolean isNewEvent(LockWatchEvent event) {
 
     private synchronized void assertNoSnapshotsMissing(Multimap<Sequence, StartTimestamp> reversedMap) {
         Set<Sequence> sequences = reversedMap.keySet();
-        if (sequences.stream().map(snapshotStore::getSnapshotForSequence).anyMatch(Optional::isEmpty)) {
+        if (sequences.stream().map(this::getSnapshotForSequence).anyMatch(Optional::isEmpty)) {
             log.warn(
                     "snapshots were not taken for all sequences; logging additional information",
                     SafeArg.of("numSequences", sequences),
@@ -260,6 +260,10 @@ private synchronized void assertNoSnapshotsMissing(Multimap<Sequence, StartTimes
         }
     }
 
+    private synchronized Optional<ValueCacheSnapshot> getSnapshotForSequence(Sequence sequence) {
+        return snapshotStore.getSnapshotForSequence(sequence);
+    }
+
     private synchronized void updateCurrentVersion(Optional<LockWatchVersion> maybeUpdateVersion) {
         maybeUpdateVersion
                 .filter(this::shouldUpdateVersion)

atlasdb-service/src/main/java/com/palantir/atlasdb/proto/fork/ForkedJsonFormat.java

@@ -1240,7 +1240,7 @@ private static String escapeText(String input) {
                     break;
                 default:
                     // Check for other control characters
-                    if (c >= 0x0000 && c <= 0x001F) {
+                    if (c <= 0x001F) {
                         appendEscapedUnicode(builder, c);
                     } else if (Character.isHighSurrogate(c)) {
                         // Encode the surrogate pair using 2 six-character sequence (\\uXXXX\\uXXXX)

atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/schema/stream/StreamTest.java

@@ -657,13 +657,15 @@ public void startFirstAndFail(Transaction tx, long streamId) {
             @Override
             public void startSecondAndFinish(Transaction tx, long streamId) {
                 StreamTestStreamStore ss = StreamTestStreamStore.of(txManager, StreamTestTableFactory.of());
-                ss.storeStreams(tx, ImmutableMap.of(streamId, new ByteArrayInputStream(new byte[1])));
+                ss.storeStreams(tx, ImmutableMap.of(streamId, new ByteArrayInputStream(new byte[] {0x42})));
             }
         });
 
         Optional<InputStream> stream = getStream(streamId);
         assertThat(stream).isPresent();
-        assertThat(stream.get()).isNotNull();
+        try (InputStream inputStream = stream.get()) {
+            assertThat(inputStream).isNotNull().hasBinaryContent(new byte[] {0x42});
+        }
     }
 
     @Test

build.gradle

@@ -9,7 +9,7 @@ buildscript {
     dependencies {
         classpath 'com.netflix.nebula:gradle-info-plugin:12.1.6'
         classpath 'com.netflix.nebula:nebula-publishing-plugin:20.3.0'
-        classpath 'com.palantir.baseline:gradle-baseline-java:4.188.0'
+        classpath 'com.palantir.baseline:gradle-baseline-java:5.18.0'
         classpath 'com.palantir.gradle.conjure:gradle-conjure:5.41.0'
         classpath 'com.palantir.gradle.consistentversions:gradle-consistent-versions:2.15.0'
         classpath 'com.palantir.gradle.docker:gradle-docker:0.32.0'

commons-db/src/main/java/com/palantir/nexus/db/sql/SQLString.java

@@ -33,6 +33,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
 import org.apache.commons.lang3.Validate;
 
@@ -86,6 +87,7 @@ protected interface CallableCheckedException<T, E extends Exception> {
      * Runs the provided callable while holding the lock for the override caches.
      * Callers replacing the caches should hold this lock.
      */
+    @Deprecated
     protected static <T, E extends Exception> T runWithCacheLock(CallableCheckedException<T, E> callable) throws E {
         synchronized (cacheLock) {
             return callable.call();
@@ -165,13 +167,12 @@ public static boolean isQueryRegistered(String key) {
      * @param dbType Look for queries registered with this override first
      * @return a SQLString object representing the stored query
      */
-    @SuppressWarnings("GuardedByChecker")
     static FinalSQLString getByKey(final String key, DBType dbType) {
         assert isValidKey(key) : "Keys only consist of word characters"; // $NON-NLS-1$
         assert registeredValues.containsKey(key) || registeredValuesOverride.containsKey(key)
                 : "Couldn't find SQLString key: " + key + ", dbtype " + dbType; // $NON-NLS-1$ //$NON-NLS-2$
 
-        FinalSQLString cached = cachedKeyed.get(key);
+        FinalSQLString cached = getCachedSql(key);
         if (null != cached) {
             callbackOnUse.noteUse((SQLString) cached.delegate);
             return cached;
@@ -192,6 +193,13 @@ static FinalSQLString getByKey(final String key, DBType dbType) {
         return valueForKey;
     }
 
+    @Nullable
+    @SuppressWarnings("GuardedBy") // we're only doing a volatile read of current cache
+    private static FinalSQLString getCachedSql(String key) {
+        ImmutableMap<String, FinalSQLString> cache = cachedKeyed; // volatile read
+        return cache == null ? null : cache.get(key);
+    }
+
     static FinalSQLString getByKey(String key, Connection connection) throws PalantirSqlException {
         DBType type = DBType.getTypeFromConnection(connection);
         return getByKey(key, type);
@@ -209,7 +217,6 @@ public static boolean isValidKey(final String key) {
      * @param sql The string to be used in a query
      * @return a SQLString object representing the given SQL
      */
-    @SuppressWarnings("GuardedByChecker")
     static FinalSQLString getUnregisteredQuery(String sql) {
         assert !isValidKey(sql) : "Unregistered Queries should not look like keys"; // $NON-NLS-1$
         return new FinalSQLString(new SQLString(sql));
@@ -423,9 +430,10 @@ protected static ImmutableMap<String, FinalSQLString> getCachedUnregistered() {
     @Deprecated
     protected static void setCachedUnregistered(ImmutableMap<String, FinalSQLString> _cachedUnregistered) {}
 
-    @SuppressWarnings("GuardedByChecker")
     protected static ImmutableMap<String, FinalSQLString> getCachedKeyed() {
-        return cachedKeyed;
+        synchronized (cacheLock) {
+            return cachedKeyed;
+        }
     }
 
     protected static void setCachedKeyed(ImmutableMap<String, FinalSQLString> cachedKeyed) {

lock-api/src/main/java/com/palantir/lock/client/LockRefreshingLockService.java

@@ -164,7 +164,8 @@ public boolean unlockAndFreeze(HeldLocksToken token) {
     }
 
     @Override
-    @SuppressWarnings("checkstyle:NoFinalizer") // TODO (jkong): Can we safely remove this without breaking things?
+    @SuppressWarnings({"checkstyle:NoFinalizer", "Finalize"}) // TODO (jkong): Can we safely remove this without
+    // breaking things?
     protected void finalize() throws Throwable {
         super.finalize();
         if (!isClosed) {

lock-api/src/main/java/com/palantir/lock/client/LockRefreshingRemoteLockService.java

@@ -123,7 +123,8 @@ private void refreshLocks() {
     }
 
     @Override
-    @SuppressWarnings("checkstyle:NoFinalizer") // TODO (jkong): Can we safely remove this without breaking things?
+    @SuppressWarnings({"checkstyle:NoFinalizer", "Finalize"})
+    // TODO (jkong): Can we safely remove this without breaking things?
     protected void finalize() throws Throwable {
         super.finalize();
         if (!isClosed) {

lock-impl/src/main/java/com/palantir/lock/logger/LockDescriptorMapper.java

@@ -40,8 +40,7 @@ ObfuscatedLockDescriptor getDescriptorMapping(LockDescriptor descriptor) {
     }
 
     /**
-     *
-     * @return map from mapping to real descriptor.
+     * Return a map from mapping to real descriptor.
      */
     Map<ObfuscatedLockDescriptor, LockDescriptor> getReversedMapper() {
         return mapper.entrySet().stream().collect(Collectors.toMap(Map.Entry::getValue, Map.Entry::getKey));