Skip to content

Commit

Permalink
Use J2ObjC's @RetainedWith annotation instead of @weak on ImmutableMa…
Browse files Browse the repository at this point in the history
…p.values

to prevent crashes on iOS.

TESTED=Created a map inside an autorelease pool and saved a reference to the
values collection. Checked that the values collection functions correctly
outside the autorelease pool. Checked that both the values collection and the
map are deallocated by the autorelease pool when not holding a strong reference
to the values collection. Did this test with ImmutableMap.of(<2 entries>) and
ImmutableMap.copyOf(<EnumMap with 2 entries>).

RELNOTES=J2ObjC: Fixes crashes from use of `ImmutableMap.values()`.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=235206171
  • Loading branch information
kstanger authored and ronshapiro committed Feb 26, 2019
1 parent ead2404 commit f4ed20e
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ public K next() {
};
}

@LazyInit private transient ImmutableCollection<V> values;
@LazyInit @RetainedWith private transient ImmutableCollection<V> values;

/**
* Returns an immutable collection of the values in this map, in the same order that they appear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import com.google.common.annotations.GwtCompatible;
import com.google.common.annotations.GwtIncompatible;
import com.google.j2objc.annotations.Weak;
import java.io.Serializable;
import java.util.Map.Entry;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
Expand All @@ -31,7 +30,7 @@
*/
@GwtCompatible(emulated = true)
final class ImmutableMapValues<K, V> extends ImmutableCollection<V> {
@Weak private final ImmutableMap<K, V> map;
private final ImmutableMap<K, V> map;

ImmutableMapValues(ImmutableMap<K, V> map) {
this.map = map;
Expand Down
2 changes: 1 addition & 1 deletion guava/src/com/google/common/collect/ImmutableMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ Spliterator<K> keySpliterator() {
return CollectSpliterators.map(entrySet().spliterator(), Entry::getKey);
}

@LazyInit private transient ImmutableCollection<V> values;
@LazyInit @RetainedWith private transient ImmutableCollection<V> values;

/**
* Returns an immutable collection of the values in this map, in the same order that they appear
Expand Down
3 changes: 1 addition & 2 deletions guava/src/com/google/common/collect/ImmutableMapValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import com.google.common.annotations.GwtCompatible;
import com.google.common.annotations.GwtIncompatible;
import com.google.j2objc.annotations.Weak;
import java.io.Serializable;
import java.util.Map.Entry;
import java.util.Spliterator;
Expand All @@ -35,7 +34,7 @@
*/
@GwtCompatible(emulated = true)
final class ImmutableMapValues<K, V> extends ImmutableCollection<V> {
@Weak private final ImmutableMap<K, V> map;
private final ImmutableMap<K, V> map;

ImmutableMapValues(ImmutableMap<K, V> map) {
this.map = map;
Expand Down
3 changes: 1 addition & 2 deletions guava/src/com/google/common/collect/RegularImmutableMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMapEntry.NonTerminalImmutableMapEntry;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.j2objc.annotations.Weak;
import java.io.Serializable;
import java.util.function.BiConsumer;
import org.checkerframework.checker.nullness.qual.Nullable;
Expand Down Expand Up @@ -264,7 +263,7 @@ ImmutableCollection<V> createValues() {

@GwtCompatible(emulated = true)
private static final class Values<K, V> extends ImmutableList<V> {
@Weak final RegularImmutableMap<K, V> map;
final RegularImmutableMap<K, V> map;

Values(RegularImmutableMap<K, V> map) {
this.map = map;
Expand Down

0 comments on commit f4ed20e

Please sign in to comment.