Skip to content

Commit 1df87a8

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Improve racy single-check init code:
- Use `@LazyInit` (to help J2ObjC and TSAN) on various `@CheckForNull transient` fields (and also on `UnmodifiableBiMap.inverse` and on the `LocalCache` fields, all of which are `@RetainedWith @CheckForNull`). I skipped some other such fields, including: - `Synchronized` (which already uses, well, `synchronized`, making the check not racy) - `TreeBasedTable.TreeRow` and some `common.graph` classes (e.g., `DirectedMultiNetworkConnections`), which are doing something more sophisticated. - Update the TODO in `ImmutableSortedMap` about actually caching a view computed on the fly. It's likely that I didn't get _every_ field that deserves an annotation. It's also likely that some of the fields should additionally use `@RetainedWith` or similar for J2ObjC purposes. And probably some fields would be better off with either eagerly computing the views or with not caching them at all (b/280079296). But this CL should be strictly an improvement over the status quo. RELNOTES=n/a PiperOrigin-RevId: 543532432
1 parent 80c8d9c commit 1df87a8

39 files changed

+142
-101
lines changed

android/guava/src/com/google/common/cache/LocalCache.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import com.google.common.util.concurrent.Uninterruptibles;
5353
import com.google.errorprone.annotations.CanIgnoreReturnValue;
5454
import com.google.errorprone.annotations.concurrent.GuardedBy;
55+
import com.google.errorprone.annotations.concurrent.LazyInit;
5556
import com.google.j2objc.annotations.RetainedWith;
5657
import com.google.j2objc.annotations.Weak;
5758
import java.io.IOException;
@@ -4201,7 +4202,7 @@ void invalidateAll(Iterable<?> keys) {
42014202
}
42024203
}
42034204

4204-
@RetainedWith @CheckForNull Set<K> keySet;
4205+
@LazyInit @RetainedWith @CheckForNull Set<K> keySet;
42054206

42064207
@Override
42074208
public Set<K> keySet() {
@@ -4210,7 +4211,7 @@ public Set<K> keySet() {
42104211
return (ks != null) ? ks : (keySet = new KeySet());
42114212
}
42124213

4213-
@RetainedWith @CheckForNull Collection<V> values;
4214+
@LazyInit @RetainedWith @CheckForNull Collection<V> values;
42144215

42154216
@Override
42164217
public Collection<V> values() {
@@ -4219,7 +4220,7 @@ public Collection<V> values() {
42194220
return (vs != null) ? vs : (values = new Values());
42204221
}
42214222

4222-
@RetainedWith @CheckForNull Set<Entry<K, V>> entrySet;
4223+
@LazyInit @RetainedWith @CheckForNull Set<Entry<K, V>> entrySet;
42234224

42244225
@Override
42254226
@GwtIncompatible // Not supported.

android/guava/src/com/google/common/collect/AbstractBiMap.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.annotations.J2ktIncompatible;
2626
import com.google.common.base.Objects;
2727
import com.google.errorprone.annotations.CanIgnoreReturnValue;
28+
import com.google.errorprone.annotations.concurrent.LazyInit;
2829
import com.google.j2objc.annotations.RetainedWith;
2930
import com.google.j2objc.annotations.WeakOuter;
3031
import java.io.IOException;
@@ -206,7 +207,7 @@ public BiMap<V, K> inverse() {
206207
return inverse;
207208
}
208209

209-
@CheckForNull private transient Set<K> keySet;
210+
@LazyInit @CheckForNull private transient Set<K> keySet;
210211

211212
@Override
212213
public Set<K> keySet() {
@@ -251,7 +252,7 @@ public Iterator<K> iterator() {
251252
}
252253
}
253254

254-
@CheckForNull private transient Set<V> valueSet;
255+
@LazyInit @CheckForNull private transient Set<V> valueSet;
255256

256257
@Override
257258
public Set<V> values() {
@@ -294,7 +295,7 @@ public String toString() {
294295
}
295296
}
296297

297-
@CheckForNull private transient Set<Entry<K, V>> entrySet;
298+
@LazyInit @CheckForNull private transient Set<Entry<K, V>> entrySet;
298299

299300
@Override
300301
public Set<Entry<K, V>> entrySet() {

android/guava/src/com/google/common/collect/AbstractSortedMultiset.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.base.Preconditions.checkNotNull;
1818

1919
import com.google.common.annotations.GwtCompatible;
20+
import com.google.errorprone.annotations.concurrent.LazyInit;
2021
import com.google.j2objc.annotations.WeakOuter;
2122
import java.util.Comparator;
2223
import java.util.Iterator;
@@ -122,7 +123,7 @@ Iterator<E> descendingIterator() {
122123
return Multisets.iteratorImpl(descendingMultiset());
123124
}
124125

125-
@CheckForNull private transient SortedMultiset<E> descendingMultiset;
126+
@LazyInit @CheckForNull private transient SortedMultiset<E> descendingMultiset;
126127

127128
@Override
128129
public SortedMultiset<E> descendingMultiset() {

android/guava/src/com/google/common/collect/ArrayTable.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.common.collect.Maps.IteratorBasedAbstractMap;
2828
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2929
import com.google.errorprone.annotations.DoNotCall;
30+
import com.google.errorprone.annotations.concurrent.LazyInit;
3031
import com.google.j2objc.annotations.WeakOuter;
3132
import java.io.Serializable;
3233
import java.lang.reflect.Array;
@@ -648,7 +649,7 @@ public ImmutableSet<C> columnKeySet() {
648649
return columnKeyToIndex.keySet();
649650
}
650651

651-
@CheckForNull private transient ColumnMap columnMap;
652+
@LazyInit @CheckForNull private transient ColumnMap columnMap;
652653

653654
@Override
654655
public Map<C, Map<R, @Nullable V>> columnMap() {
@@ -743,7 +744,7 @@ public ImmutableSet<R> rowKeySet() {
743744
return rowKeyToIndex.keySet();
744745
}
745746

746-
@CheckForNull private transient RowMap rowMap;
747+
@LazyInit @CheckForNull private transient RowMap rowMap;
747748

748749
@Override
749750
public Map<R, Map<C, @Nullable V>> rowMap() {

android/guava/src/com/google/common/collect/CompactHashMap.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.common.base.Preconditions;
3131
import com.google.common.primitives.Ints;
3232
import com.google.errorprone.annotations.CanIgnoreReturnValue;
33+
import com.google.errorprone.annotations.concurrent.LazyInit;
3334
import com.google.j2objc.annotations.WeakOuter;
3435
import java.io.IOException;
3536
import java.io.InvalidObjectException;
@@ -671,7 +672,7 @@ private void checkForConcurrentModification() {
671672
}
672673
}
673674

674-
@CheckForNull private transient Set<K> keySetView;
675+
@LazyInit @CheckForNull private transient Set<K> keySetView;
675676

676677
@Override
677678
public Set<K> keySet() {
@@ -727,7 +728,7 @@ K getOutput(int entry) {
727728
};
728729
}
729730

730-
@CheckForNull private transient Set<Entry<K, V>> entrySetView;
731+
@LazyInit @CheckForNull private transient Set<Entry<K, V>> entrySetView;
731732

732733
@Override
733734
public Set<Entry<K, V>> entrySet() {
@@ -907,7 +908,7 @@ public boolean containsValue(@CheckForNull Object value) {
907908
return false;
908909
}
909910

910-
@CheckForNull private transient Collection<V> valuesView;
911+
@LazyInit @CheckForNull private transient Collection<V> valuesView;
911912

912913
@Override
913914
public Collection<V> values() {

android/guava/src/com/google/common/collect/DescendingMultiset.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.common.collect;
1818

1919
import com.google.common.annotations.GwtCompatible;
20+
import com.google.errorprone.annotations.concurrent.LazyInit;
2021
import com.google.j2objc.annotations.WeakOuter;
2122
import java.util.Comparator;
2223
import java.util.Iterator;
@@ -37,7 +38,7 @@ abstract class DescendingMultiset<E extends @Nullable Object> extends Forwarding
3738
implements SortedMultiset<E> {
3839
abstract SortedMultiset<E> forwardMultiset();
3940

40-
@CheckForNull private transient Comparator<? super E> comparator;
41+
@LazyInit @CheckForNull private transient Comparator<? super E> comparator;
4142

4243
@Override
4344
public Comparator<? super E> comparator() {
@@ -48,7 +49,7 @@ public Comparator<? super E> comparator() {
4849
return result;
4950
}
5051

51-
@CheckForNull private transient NavigableSet<E> elementSet;
52+
@LazyInit @CheckForNull private transient NavigableSet<E> elementSet;
5253

5354
@Override
5455
public NavigableSet<E> elementSet() {
@@ -116,7 +117,7 @@ public Entry<E> lastEntry() {
116117

117118
abstract Iterator<Entry<E>> entryIterator();
118119

119-
@CheckForNull private transient Set<Entry<E>> entrySet;
120+
@LazyInit @CheckForNull private transient Set<Entry<E>> entrySet;
120121

121122
@Override
122123
public Set<Entry<E>> entrySet() {

android/guava/src/com/google/common/collect/GeneralRange.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import com.google.common.annotations.GwtCompatible;
2424
import com.google.common.base.Objects;
25+
import com.google.errorprone.annotations.concurrent.LazyInit;
2526
import java.io.Serializable;
2627
import java.util.Comparator;
2728
import javax.annotation.CheckForNull;
@@ -263,7 +264,7 @@ public int hashCode() {
263264
getUpperBoundType());
264265
}
265266

266-
@CheckForNull private transient GeneralRange<T> reverse;
267+
@LazyInit @CheckForNull private transient GeneralRange<T> reverse;
267268

268269
/** Returns the same range relative to the reversed comparator. */
269270
GeneralRange<T> reverse() {

android/guava/src/com/google/common/collect/ImmutableRangeSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ private final class AsSet extends ImmutableSortedSet<C> {
542542
this.domain = domain;
543543
}
544544

545-
@CheckForNull private transient Integer size;
545+
@LazyInit @CheckForNull private transient Integer size;
546546

547547
@Override
548548
public int size() {

android/guava/src/com/google/common/collect/ImmutableSortedMap.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,16 +1083,18 @@ public final Entry<K, V> pollLastEntry() {
10831083

10841084
@Override
10851085
public ImmutableSortedMap<K, V> descendingMap() {
1086-
// TODO(kevinb): the descendingMap is never actually cached at all. Either it should be or the
1087-
// code below simplified.
1086+
// TODO(kevinb): The descendingMap is never actually cached at all. Either:
1087+
//
1088+
// - Cache it, and annotate the field with @LazyInit.
1089+
// - Simplify the code below, and consider eliminating the field (b/287198172), which is also
1090+
// set by one of the constructors.
10881091
ImmutableSortedMap<K, V> result = descendingMap;
10891092
if (result == null) {
10901093
if (isEmpty()) {
1091-
return result = emptyMap(Ordering.from(comparator()).reverse());
1094+
return emptyMap(Ordering.from(comparator()).reverse());
10921095
} else {
1093-
return result =
1094-
new ImmutableSortedMap<>(
1095-
(RegularImmutableSortedSet<K>) keySet.descendingSet(), valueList.reverse(), this);
1096+
return new ImmutableSortedMap<>(
1097+
(RegularImmutableSortedSet<K>) keySet.descendingSet(), valueList.reverse(), this);
10961098
}
10971099
}
10981100
return result;

android/guava/src/com/google/common/collect/MapMakerInternalMap.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.primitives.Ints;
2626
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2727
import com.google.errorprone.annotations.concurrent.GuardedBy;
28+
import com.google.errorprone.annotations.concurrent.LazyInit;
2829
import com.google.j2objc.annotations.Weak;
2930
import com.google.j2objc.annotations.WeakOuter;
3031
import java.io.IOException;
@@ -2522,23 +2523,23 @@ public void clear() {
25222523
}
25232524
}
25242525

2525-
@CheckForNull transient Set<K> keySet;
2526+
@LazyInit @CheckForNull transient Set<K> keySet;
25262527

25272528
@Override
25282529
public Set<K> keySet() {
25292530
Set<K> ks = keySet;
25302531
return (ks != null) ? ks : (keySet = new KeySet());
25312532
}
25322533

2533-
@CheckForNull transient Collection<V> values;
2534+
@LazyInit @CheckForNull transient Collection<V> values;
25342535

25352536
@Override
25362537
public Collection<V> values() {
25372538
Collection<V> vs = values;
25382539
return (vs != null) ? vs : (values = new Values());
25392540
}
25402541

2541-
@CheckForNull transient Set<Entry<K, V>> entrySet;
2542+
@LazyInit @CheckForNull transient Set<Entry<K, V>> entrySet;
25422543

25432544
@Override
25442545
public Set<Entry<K, V>> entrySet() {

0 commit comments

Comments
 (0)