From 2983584ef6e7099a8a83ecbcde7288af6d4eeb4a Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Wed, 28 Oct 2020 10:22:47 +0100
Subject: [PATCH] Fix #invariant Assertion in CacheFile (#64180) (#64264)

Fix #invariant Assertion in CacheFile

closes #64141
---
 .../util/concurrent/AbstractRefCounted.java       |  5 +++--
 .../common/util/concurrent/RefCounted.java        |  5 +++--
 .../java/org/elasticsearch/index/store/Store.java |  4 ++--
 .../index/store/cache/CacheFile.java              | 15 ++++++++++-----
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/AbstractRefCounted.java b/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/AbstractRefCounted.java
index a30e7490ff4..7483f7a8580 100644
--- a/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/AbstractRefCounted.java
+++ b/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/AbstractRefCounted.java
@@ -56,13 +56,14 @@ public abstract class AbstractRefCounted implements RefCounted {
     }
 
     @Override
-    public final void decRef() {
+    public final boolean decRef() {
         int i = refCount.decrementAndGet();
         assert i >= 0;
         if (i == 0) {
             closeInternal();
+            return true;
         }
-
+        return false;
     }
 
     protected void alreadyClosed() {
diff --git a/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/RefCounted.java b/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/RefCounted.java
index 1e7bdc0e78f..e573ba54026 100644
--- a/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/RefCounted.java
+++ b/libs/core/src/main/java/org/elasticsearch/common/util/concurrent/RefCounted.java
@@ -61,7 +61,8 @@ public interface RefCounted {
      * instance is considered as closed and should not be used anymore.
      *
      * @see #incRef
+     *
+     * @return returns {@code true} if the ref count dropped to 0 as a result of calling this method
      */
-    void decRef();
-
+    boolean decRef();
 }
diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java
index d040e6f44c7..f60ef2facd9 100644
--- a/server/src/main/java/org/elasticsearch/index/store/Store.java
+++ b/server/src/main/java/org/elasticsearch/index/store/Store.java
@@ -406,8 +406,8 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref
      * @see #incRef
      */
     @Override
-    public final void decRef() {
-        refCounter.decRef();
+    public final boolean decRef() {
+        return refCounter.decRef();
     }
 
     @Override
diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CacheFile.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CacheFile.java
index 020eb3832f4..72287944f46 100644
--- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CacheFile.java
+++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CacheFile.java
@@ -51,6 +51,7 @@ public class CacheFile {
     private final AbstractRefCounted refCounter = new AbstractRefCounted("CacheFile") {
         @Override
         protected void closeInternal() {
+            assert evicted.get();
             assert assertNoPendingListeners();
             try {
                 Files.deleteIfExists(file);
@@ -95,7 +96,7 @@ public class CacheFile {
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
             } finally {
-                refCounter.decRef();
+                decrementRefCount();
             }
         }
     }
@@ -147,7 +148,7 @@ public class CacheFile {
                 success = true;
             } finally {
                 if (success == false) {
-                    refCounter.decRef();
+                    decrementRefCount();
                 }
             }
         }
@@ -175,7 +176,7 @@ public class CacheFile {
             success = true;
         } finally {
             if (success) {
-                refCounter.decRef();
+                decrementRefCount();
             }
         }
         assert invariant();
@@ -190,6 +191,11 @@ public class CacheFile {
         return true;
     }
 
+    private void decrementRefCount() {
+        final boolean released = refCounter.decRef();
+        assert released == false || Files.notExists(file);
+    }
+
     /**
      * Evicts this file from the cache. Once this method has been called, subsequent use of this class with throw exceptions.
      */
@@ -199,7 +205,7 @@ public class CacheFile {
             synchronized (listeners) {
                 evictionListeners = new HashSet<>(listeners);
             }
-            refCounter.decRef();
+            decrementRefCount();
             evictionListeners.forEach(listener -> listener.onEviction(this));
         }
         assert invariant();
@@ -209,7 +215,6 @@ public class CacheFile {
         synchronized (listeners) {
             if (listeners.isEmpty()) {
                 assert channelRef == null;
-                assert evicted.get() == false || refCounter.refCount() != 0 || Files.notExists(file);
             } else {
                 assert channelRef != null;
                 assert refCounter.refCount() > 0;