Skip to content

Commit

Permalink
Recommend Cleaner instead of FinalizableReferenceQueue.
Browse files Browse the repository at this point in the history 
Also add a test that validates that the example given for FRQ in its doc comment are sound.

We're not deprecating FRQ yet, because `Cleaner` is still fairly recent in the Android world.

RELNOTES=We now recommend the standard `Cleaner` class instead of `FinalizableReferenceQueue`, when `Cleaner` is available.
PiperOrigin-RevId: 705490132
  • Loading branch information
@eamonnmcmanus
eamonnmcmanus authored and Google Java Core Libraries committed Dec 12, 2024
1 parent e307cae commit 62c6187
Show file tree
Hide file tree
Showing 4 changed files with 266 additions and 46 deletions.
103 changes: 83 additions & 20 deletions android/guava-tests/test/com/google/common/base/FinalizableReferenceQueueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,28 @@

package com.google.common.base;

import static com.google.common.truth.Truth.assertThat;

import com.google.common.annotations.GwtIncompatible;
import com.google.common.base.internal.Finalizer;
import com.google.common.collect.Sets;
import com.google.common.testing.GcFinalization;
import java.io.Closeable;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.net.ServerSocket;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.Arrays;
import java.util.Collections;
import junit.framework.TestCase;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Unit test for {@link FinalizableReferenceQueue}.
Expand All @@ -38,26 +49,21 @@
// - possibly no real concept of separate ClassLoaders?
@AndroidIncompatible
@GwtIncompatible
public class FinalizableReferenceQueueTest extends TestCase {
@RunWith(JUnit4.class)
public class FinalizableReferenceQueueTest {

private @Nullable FinalizableReferenceQueue frq;

@Override
protected void tearDown() throws Exception {
@After
public void tearDown() throws Exception {
frq = null;
}


@Test
public void testFinalizeReferentCalled() {
final MockReference reference = new MockReference(frq = new FinalizableReferenceQueue());

GcFinalization.awaitDone(
new GcFinalization.FinalizationPredicate() {
@Override
public boolean isDone() {
return reference.finalizeReferentCalled;
}
});
GcFinalization.awaitDone(() -> reference.finalizeReferentCalled);
}

static class MockReference extends FinalizableWeakReference<Object> {
Expand All @@ -80,7 +86,7 @@ public void finalizeReferent() {
*/
private WeakReference<ReferenceQueue<Object>> queueReference;


@Test
public void testThatFinalizerStops() {
weaklyReferenceQueue();
GcFinalization.awaitClear(queueReference);
Expand Down Expand Up @@ -109,6 +115,7 @@ public void finalizeReferent() {
};
}

@Test
public void testDecoupledLoader() {
FinalizableReferenceQueue.DecoupledLoader decoupledLoader =
new FinalizableReferenceQueue.DecoupledLoader() {
Expand All @@ -120,10 +127,10 @@ URLClassLoader newLoader(URL base) {

Class<?> finalizerCopy = decoupledLoader.loadFinalizer();

assertNotNull(finalizerCopy);
assertNotSame(Finalizer.class, finalizerCopy);
assertThat(finalizerCopy).isNotNull();
assertThat(finalizerCopy).isNotSameInstanceAs(Finalizer.class);

assertNotNull(FinalizableReferenceQueue.getStartFinalizer(finalizerCopy));
assertThat(FinalizableReferenceQueue.getStartFinalizer(finalizerCopy)).isNotNull();
}

static class DecoupledClassLoader extends URLClassLoader {
Expand All @@ -148,13 +155,69 @@ protected synchronized Class<?> loadClass(String name, boolean resolve)
}
}

@Test
public void testGetFinalizerUrl() {
assertNotNull(getClass().getResource("internal/Finalizer.class"));
assertThat(getClass().getResource("internal/Finalizer.class")).isNotNull();
}

@Test
public void testFinalizeClassHasNoNestedClasses() throws Exception {
// Ensure that the Finalizer class has no nested classes.
// See https://github.com/google/guava/issues/1505
assertEquals(Collections.emptyList(), Arrays.asList(Finalizer.class.getDeclaredClasses()));
assertThat(Finalizer.class.getDeclaredClasses()).isEmpty();
}

static class MyServerExampleWithFrq implements Closeable {
private static final FinalizableReferenceQueue frq = new FinalizableReferenceQueue();

private static final Set<Reference<?>> references = Sets.newConcurrentHashSet();

private final ServerSocket serverSocket;

private MyServerExampleWithFrq() throws IOException {
this.serverSocket = new ServerSocket(0);
}

static MyServerExampleWithFrq create(AtomicBoolean finalizeReferentRan) throws IOException {
MyServerExampleWithFrq myServer = new MyServerExampleWithFrq();
ServerSocket serverSocket = myServer.serverSocket;
Reference<?> reference =
new FinalizablePhantomReference<MyServerExampleWithFrq>(myServer, frq) {
@Override
public void finalizeReferent() {
references.remove(this);
if (!serverSocket.isClosed()) {
finalizeReferentRan.set(true);
try {
serverSocket.close();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
}
};
references.add(reference);
return myServer;
}

@Override
public void close() throws IOException {
serverSocket.close();
}
}

private ServerSocket makeMyServerExampleWithFrq(AtomicBoolean finalizeReferentRan)
throws IOException {
MyServerExampleWithFrq myServer = MyServerExampleWithFrq.create(finalizeReferentRan);
assertThat(myServer.serverSocket.isClosed()).isFalse();
return myServer.serverSocket;
}

@Test
public void testMyServerExampleWithFrq() throws Exception {
AtomicBoolean finalizeReferentRan = new AtomicBoolean(false);
ServerSocket serverSocket = makeMyServerExampleWithFrq(finalizeReferentRan);
GcFinalization.awaitDone(finalizeReferentRan::get);
assertThat(serverSocket.isClosed()).isTrue();
}
}
53 changes: 50 additions & 3 deletions android/guava/src/com/google/common/base/FinalizableReferenceQueue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

/**
* A reference queue with an associated background thread that dequeues references and invokes
* {@link FinalizableReference#finalizeReferent()} on them.
* {@link FinalizableReference#finalizeReferent()} on them. Java 9+ users should prefer {@link
* java.lang.ref.Cleaner Cleaner}; see example <a href="#cleaner">below</a>.
*
* <p>Keep a strong reference to this object until all of the associated referents have been
* finalized. If this object is garbage collected earlier, the backing thread will not invoke {@code
Expand Down Expand Up @@ -62,8 +63,9 @@
*
* public static MyServer create(...) {
* MyServer myServer = new MyServer(...);
* final ServerSocket serverSocket = myServer.serverSocket;
* ServerSocket serverSocket = myServer.serverSocket;
* Reference<?> reference = new FinalizablePhantomReference<MyServer>(myServer, frq) {
* @Override
* public void finalizeReferent() {
* references.remove(this):
* if (!serverSocket.isClosed()) {
Expand All @@ -80,12 +82,57 @@
* return myServer;
* }
*
* public void close() {
* @Override
* public void close() throws IOException {
* serverSocket.close();
* }
* }
* }</pre>
*
* <p id="cleaner">Here is how you might achieve the same thing using {@link java.lang.ref.Cleaner
* Cleaner}, if you are using a Java version where that is available:
*
* <pre>{@code
* public class MyServer implements Closeable {
* private static final Cleaner cleaner = Cleaner.create();
* // You might also share this between several objects.
*
* private final ServerSocket serverSocket;
* private final Cleaner.Cleanable cleanable;
*
* public MyServer(...) {
* ...
* this.serverSocket = new ServerSocket(...);
* this.cleanable = cleaner.register(this, closeServerSocketRunnable(serverSocket));
* ...
* }
*
* private static Runnable closeServerSocketRunnable(ServerSocket serverSocket) {
* return () -> {
* if (!serverSocket.isClosed()) {
* ...log a message about how nobody called close()...
* try {
* serverSocket.close();
* } catch (IOException e) {
* ...
* }
* }
* };
* }
*
* @Override
* public void close() throws IOException {
* serverSocket.close();
* cleanable.clean();
* }
* }
* }</pre>
*
* <p>Some care is needed when using {@code Cleaner} to ensure that the callback passed to {@code
* register} does not have a reference to the object (in this case, {@code MyServer}) that may be
* garbage-collected. That's why we are careful to make a {@code Runnable} that does not have a
* reference to any {@code MyServer} instance.
*
* @author Bob Lee
* @since 2.0
*/
Expand Down
Loading

0 comments on commit 62c6187

Please sign in to comment.