1

I have a buffer pool implementation which basically provides pre-allocated ByteBuffer objects via allocate()/release() API. In order to detect the cases when caller forgot to call release and the ByteBuffer ref is leaked, I am using Guava's FinalizableReferenceQueue in conjunction with FinalizablePhantomReference. finalizeReferent().

Additionally, I need to selectively destroy the buffer pool and replace it with a newer one with a different configuration. For that, I was setting previous SampleBufferPool reference to null and let garbage collector do its job. However, I noticed that the ByteBuffer were not getting collected/finalizeReferent is not being called. (I verified that the full GC pause are not collecting any memory via adding -XX:+PrintGCDetails -verbose:gc JVM flags)

package foo;

import com.google.common.base.FinalizablePhantomReference;
import com.google.common.base.FinalizableReferenceQueue;
import com.google.common.collect.Sets;
import java.lang.ref.Reference;
import java.nio.ByteBuffer;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.stream.IntStream;


public class App {
  public static class SampleBufferPool {
    // Phantom reference queue for detecting memory leaks
    // See. https://guava.dev/releases/19.0/api/docs/com/google/common/base/FinalizableReferenceQueue.html
    private static final FinalizableReferenceQueue FRQ = new FinalizableReferenceQueue();
    // This ensures that the FinalizablePhantomReference itself is not garbage-collected.
    public static final Set<Reference<?>> REFERENCES = Sets.newConcurrentHashSet();

    private final ConcurrentLinkedDeque<ByteBuffer> _bufferCache = new ConcurrentLinkedDeque<>();
    private final int _chunkSize;
    private final int _numChunks;

    public SampleBufferPool(int chunkSize, int numChunks) {
      _chunkSize = chunkSize;
      _numChunks = numChunks;
      IntStream.range(0, _numChunks).forEach(i -> populateSingleChunk());
    }

    public ByteBuffer allocate() {
      return _bufferCache.pollLast();
    }

    public void release(ByteBuffer chunk) {
      _bufferCache.offerLast(chunk);
    }

    private void populateSingleChunk() {
      ByteBuffer chunk = ByteBuffer.allocate(_chunkSize);

      _bufferCache.offerLast(chunk);

      Reference<?> reference = new FinalizablePhantomReference<>(chunk, FRQ) {
        @Override
        public void finalizeReferent() {
          REFERENCES.remove(this);
          System.out.println("LEAK DETECTED. ByteBuf[" + "] from RecyclingMemoryPool");
        }
      };
      REFERENCES.add(reference);
    }
  }

  public static void main(String[] args) {
    SampleBufferPool sampleBufferPool = new SampleBufferPool(20000000, 400);
    sampleBufferPool = null;

    for (int i = 0; i < 10; i++) {
      System.gc();
    }
  }
}

Tushar Goyal
  • 212
  • 3
  • 15

1 Answers1

3

You are creating a subclass of FinalizablePhantomReference as anonymous subclass inside a non-static context:

Reference<?> reference = new FinalizablePhantomReference<>(chunk, FRQ) {
  @Override
  public void finalizeReferent() {
    REFERENCES.remove(this);
    System.out.println("LEAK DETECTED. ByteBuf[" + "] from RecyclingMemoryPool");
  }
};

Prior to JDK 18, anonymous inner classes always keep a reference to their surrounding instance, whether they are using it or not. As described by bug report JDK-8271717, this changes with JDK 18 when compiling the source code with javac. Since this still is a compiler specific behavior and further, it is too easy to use a member of the surrounding class by accident, which would force keeping a reference to the instance, you should use a static context. E.g.

private void populateSingleChunk() {
  ByteBuffer chunk = ByteBuffer.allocate(_chunkSize);

  _bufferCache.offerLast(chunk);

  registerChunk(chunk);
}
private static void registerChunk(ByteBuffer chunk) {
  Reference<?> reference = new FinalizablePhantomReference<>(chunk, FRQ) {
    @Override
    public void finalizeReferent() {
      REFERENCES.remove(this);
      System.out.println("LEAK DETECTED. ByteBuf[" + "] from RecyclingMemoryPool");
    }
  };
  REFERENCES.add(reference);
}

Of course, the phantom reference object must not have strong references to the referent as well. If you need properties of the referent, you must extract them beforehand, e.g.

private void populateSingleChunk() {
  ByteBuffer chunk = ByteBuffer.allocate(_chunkSize);

  _bufferCache.offerLast(chunk);

  registerChunk(chunk);
}
private static void registerChunk(ByteBuffer chunk) {
  int capacity = chunk.capacity();
  Reference<?> reference = new FinalizablePhantomReference<>(chunk, FRQ) {
    @Override
    public void finalizeReferent() {
      REFERENCES.remove(this);
      System.out.println("LEAK DETECTED. ByteBuf[capacity = "
                       + capacity + "] from RecyclingMemoryPool");
    }
  };
  REFERENCES.add(reference);
}
Holger
  • 285,553
  • 42
  • 434
  • 765
  • I think having a static context is not the full solution here. I botched the log line when I originally posted and meant to do `System.out.println("LEAK DETECTED. ByteBuf[" + chunk.capacity() + "] from RecyclingMemoryPool");`. In this case, it still doesn't work. IIUC it's because the chunk is still being referred by PhantomReference object itself (in the log line). WDYT if I simply make the `REFERENCES` non-static member since I don't care about the underlying ByteBuffer in this case anymore? Prevents the dead reference accumulation in the Set and onus of clearing them – Tushar Goyal May 11 '22 at 18:28
  • Tagging @holger. Think, the user will get notified by default, just for redundancy – Tushar Goyal May 11 '22 at 20:25
  • 1
    Tagging is indeed not necessary when commenting an answer. Of course, I can only answer to the code given in the question. I expanded the answer to cover your new information. Making the `REFERENCES` set non-static is not helpful, it would allow the garbage collection of the object *and* the phantom reference, [so it never gets enqueued](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/ref/package-summary.html#:~:text=The%20relationship%20between%20a%20registered%20reference%20object%20and%20its%20queue%20is%20one%2Dsided.), hence you wouldn’t get any notification. – Holger May 12 '22 at 07:58