Fix #invariant Assertion in CacheFile (#64180) (#64264)

Fix #invariant Assertion in CacheFile

closes #64141
This commit is contained in:
Armin Braun 2020-10-28 10:22:47 +01:00 committed by GitHub
parent a697d5edae
commit 2983584ef6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 18 additions and 11 deletions

View File

@ -56,13 +56,14 @@ public abstract class AbstractRefCounted implements RefCounted {
} }
@Override @Override
public final void decRef() { public final boolean decRef() {
int i = refCount.decrementAndGet(); int i = refCount.decrementAndGet();
assert i >= 0; assert i >= 0;
if (i == 0) { if (i == 0) {
closeInternal(); closeInternal();
return true;
} }
return false;
} }
protected void alreadyClosed() { protected void alreadyClosed() {

View File

@ -61,7 +61,8 @@ public interface RefCounted {
* instance is considered as closed and should not be used anymore. * instance is considered as closed and should not be used anymore.
* *
* @see #incRef * @see #incRef
*
* @return returns {@code true} if the ref count dropped to 0 as a result of calling this method
*/ */
void decRef(); boolean decRef();
} }

View File

@ -406,8 +406,8 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref
* @see #incRef * @see #incRef
*/ */
@Override @Override
public final void decRef() { public final boolean decRef() {
refCounter.decRef(); return refCounter.decRef();
} }
@Override @Override

View File

@ -51,6 +51,7 @@ public class CacheFile {
private final AbstractRefCounted refCounter = new AbstractRefCounted("CacheFile") { private final AbstractRefCounted refCounter = new AbstractRefCounted("CacheFile") {
@Override @Override
protected void closeInternal() { protected void closeInternal() {
assert evicted.get();
assert assertNoPendingListeners(); assert assertNoPendingListeners();
try { try {
Files.deleteIfExists(file); Files.deleteIfExists(file);
@ -95,7 +96,7 @@ public class CacheFile {
} catch (IOException e) { } catch (IOException e) {
throw new UncheckedIOException(e); throw new UncheckedIOException(e);
} finally { } finally {
refCounter.decRef(); decrementRefCount();
} }
} }
} }
@ -147,7 +148,7 @@ public class CacheFile {
success = true; success = true;
} finally { } finally {
if (success == false) { if (success == false) {
refCounter.decRef(); decrementRefCount();
} }
} }
} }
@ -175,7 +176,7 @@ public class CacheFile {
success = true; success = true;
} finally { } finally {
if (success) { if (success) {
refCounter.decRef(); decrementRefCount();
} }
} }
assert invariant(); assert invariant();
@ -190,6 +191,11 @@ public class CacheFile {
return true; 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. * 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) { synchronized (listeners) {
evictionListeners = new HashSet<>(listeners); evictionListeners = new HashSet<>(listeners);
} }
refCounter.decRef(); decrementRefCount();
evictionListeners.forEach(listener -> listener.onEviction(this)); evictionListeners.forEach(listener -> listener.onEviction(this));
} }
assert invariant(); assert invariant();
@ -209,7 +215,6 @@ public class CacheFile {
synchronized (listeners) { synchronized (listeners) {
if (listeners.isEmpty()) { if (listeners.isEmpty()) {
assert channelRef == null; assert channelRef == null;
assert evicted.get() == false || refCounter.refCount() != 0 || Files.notExists(file);
} else { } else {
assert channelRef != null; assert channelRef != null;
assert refCounter.refCount() > 0; assert refCounter.refCount() > 0;