From 0ad38b88bd92319508ebc0b28bab5be1fbf1891f Mon Sep 17 00:00:00 2001 From: micapolos Date: Tue, 23 Jun 2020 06:58:15 -0700 Subject: [PATCH] Fix memory leak in SingletonImmutableBiMap which would appear in transpiled J2ObjC code. The @RetainedWith annotation can not be used on both sides of the retain-cycle. The RegularImmutableBiMap does not have this problem, because it uses Inverse inner class. This CL applies similar trick in SingletonImmutableBiMap, although without additional inner class. RELNOTES=Update SingletonImmutableBiMap to avoid retain-cycle in transpiled Obj-C code. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=317856944 --- cycle_whitelist.txt | 4 ++-- .../collect/SingletonImmutableBiMap.java | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cycle_whitelist.txt b/cycle_whitelist.txt index 56d6251b6a91..96c9e1b2aa63 100644 --- a/cycle_whitelist.txt +++ b/cycle_whitelist.txt @@ -13,8 +13,6 @@ NAMESPACE junit.framework NAMESPACE org.junit # ***** REAL CYCLES ***** -# Inverses (currently not solvable by weakening a reference) -FIELD com.google.common.base.Converter.reverse # Cycle exists until future completes FIELD com.google.common.util.concurrent.AbstractFuture.Listener.executor com.google.common.util.concurrent.ExecutionSequencer.TaskNonReentrantExecutor @@ -23,6 +21,7 @@ FIELD com.google.common.util.concurrent.AbstractFuture.Listener.executor com.goo # The Runnable type is so generic that it produces too many false positives. TYPE java.lang.Runnable +FIELD com.google.common.base.Converter.reverse FIELD com.google.common.collect.AbstractBiMap.EntrySet.iterator.$.entry com.google.common.collect.AbstractBiMap.EntrySet.iterator.$.next.$ FIELD com.google.common.collect.AbstractMapBasedMultimap.map FIELD com.google.common.collect.AbstractMultimap.asMap com.google.common.collect.AbstractMapBasedMultimap.NavigableAsMap @@ -36,6 +35,7 @@ FIELD com.google.common.collect.ImmutableRangeSet.ranges FIELD com.google.common.collect.ImmutableSet.asList FIELD com.google.common.collect.Maps.FilteredMapValues.unfiltered FIELD com.google.common.collect.Sets.SubSet.inputSet +FIELD com.google.common.collect.SingletonImmutableBiMap.inverse FIELD com.google.common.collect.TreeTraverser.PostOrderNode.childIterator FIELD com.google.common.collect.TreeTraverser.PreOrderIterator.stack FIELD com.google.common.util.concurrent.AbstractFuture.Listener.executor com.google.common.util.concurrent.MoreExecutors.rejectionPropagatingExecutor.$ diff --git a/guava/src/com/google/common/collect/SingletonImmutableBiMap.java b/guava/src/com/google/common/collect/SingletonImmutableBiMap.java index 32376e07803b..ea6f275677d3 100644 --- a/guava/src/com/google/common/collect/SingletonImmutableBiMap.java +++ b/guava/src/com/google/common/collect/SingletonImmutableBiMap.java @@ -42,6 +42,7 @@ final class SingletonImmutableBiMap extends ImmutableBiMap { checkEntryNotNull(singleKey, singleValue); this.singleKey = singleKey; this.singleValue = singleValue; + this.inverse = null; } private SingletonImmutableBiMap(K singleKey, V singleValue, ImmutableBiMap inverse) { @@ -90,16 +91,21 @@ ImmutableSet createKeySet() { return ImmutableSet.of(singleKey); } - @LazyInit @RetainedWith transient ImmutableBiMap inverse; + private final transient @Nullable ImmutableBiMap inverse; + @LazyInit @RetainedWith private transient @Nullable ImmutableBiMap lazyInverse; @Override public ImmutableBiMap inverse() { - // racy single-check idiom - ImmutableBiMap result = inverse; - if (result == null) { - return inverse = new SingletonImmutableBiMap<>(singleValue, singleKey, this); + if (inverse != null) { + return inverse; } else { - return result; + // racy single-check idiom + ImmutableBiMap result = lazyInverse; + if (result == null) { + return lazyInverse = new SingletonImmutableBiMap<>(singleValue, singleKey, this); + } else { + return result; + } } } }