Skip to content

Commit

Permalink
make SCQ.closeables use WeakReference so that tailers/appenders refer…
Browse files Browse the repository at this point in the history
…ences not leaked after they would otherwise be eligible for GC. Closes #1455
  • Loading branch information
JerryShea committed Jan 27, 2024
1 parent 0793ba0 commit 440307c
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.jetbrains.annotations.Nullable;

import java.io.*;
import java.lang.ref.WeakReference;
import java.security.SecureRandom;
import java.text.ParseException;
import java.time.ZoneId;
Expand Down Expand Up @@ -110,7 +111,7 @@
private final TimeProvider time;
@NotNull
private final BiFunction<RollingChronicleQueue, Wire, SingleChronicleQueueStore> storeFactory;
private final Set<Closeable> closers = Collections.newSetFromMap(new IdentityHashMap<>());
private final Set<WeakReference<Closeable>> closers = Collections.newSetFromMap(new IdentityHashMap<>());
private final boolean readOnly;
@NotNull
private final CycleCalculator cycleCalculator;
Expand Down Expand Up @@ -716,16 +717,19 @@ public NavigableSet<Long> listCyclesBetween(int lowerCycle, int upperCycle) {
public <T> void addCloseListener(Closeable key) {
synchronized (closers) {
if (!closers.isEmpty())
closers.removeIf(Closeable::isClosed);
closers.add(key);
closers.removeIf(wrc -> {
final Closeable closeable = wrc.get();
return closeable != null || closeable.isClosed();
});
closers.add(new WeakReference<>(key));
}
}

@SuppressWarnings("unchecked")
@Override
protected void performClose() {
synchronized (closers) {
metaStoreMap.values().forEach(Closeable::closeQuietly);
Closeable.closeQuietly(metaStoreMap.values());
metaStoreMap.clear();
closers.forEach(Closeable::closeQuietly);
closers.clear();
Expand All @@ -746,7 +750,7 @@ protected void performClose() {

// close it if we created it.
if (eventLoop instanceof OnDemandEventLoop)
eventLoop.close();
Closeable.closeQuietly(eventLoop);
}

@Override
Expand Down Expand Up @@ -911,9 +915,14 @@ private ToIntFunction<String> fileNameToCycleFunction() {
return name -> dateCache.parseCount(name.substring(0, name.length() - SUFFIX.length()));
}

@Deprecated(/* to be removed in x.25 */)
void removeCloseListener(final StoreTailer storeTailer) {
removeCloseListener((java.io.Closeable) storeTailer);
}

void removeCloseListener(final java.io.Closeable closeable) {
synchronized (closers) {
closers.remove(storeTailer);
closers.removeIf(wrc -> wrc.get() == closeable);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public void writeBytes(@NotNull final WriteBytesMarshallable marshallable) {

@Override
protected void performClose() {
// queue.removeCloseListener(this);
releaseBytesFor(wireForIndex);
releaseBytesFor(wire);
releaseBytesFor(bufferWire);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public DocumentContext readingDocument() {

@Override
protected void performClose() {
// queue.removeCloseListener((java.io.Closeable) this);
Closeable.closeQuietly(indexUpdater);
// the wire ref count will be released here by setting it to null
context.wire(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ public void recordExceptions() {
for (String msg : "Shrinking ,Allocation of , ms to add mapping for ,jar to the classpath, ms to pollDiskSpace for , us to linearScan by position from ,File released ,Overriding roll length from existing metadata, was 3600000, overriding to 86400000 ".split(",")) {
ignoreException(msg);
}
// See https://github.com/OpenHFT/Chronicle-Queue/issues/1455. As many unit tests do not use try/finally
// to manage tailer or appender scope properly. we are ignoring this exception for now.
// TODO: remove the below line and run the tests many times to flush out the problematic tests (or else inspect the codebase)
ignoreException("Discarded without closing");
}

public void ignoreException(String message) {
Expand Down

0 comments on commit 440307c

Please sign in to comment.