Skip to content

Commit 3298443

Browse files
authored
GH-130: Fix AutoCloseables to work with @nullable structures (#1017)
## What's Changed `AutoCloseables` supposes to work with nullable `Iterables`, `varargs`, and `collection of nulls`. The PR introduces: - `@Nullable` annotation for all public methods in `AutoCloseables` (only private `flatten` method doesn't support null `Iterable`) - `null` checks to prevent NPEs --- The change is backward compatible. Only possible NPEs are prevented. --- Closes #130 .
1 parent 9d6237e commit 3298443

File tree

2 files changed

+311
-18
lines changed

2 files changed

+311
-18
lines changed

memory/memory-core/src/main/java/org/apache/arrow/util/AutoCloseables.java

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
import java.util.Collections;
2323
import java.util.Iterator;
2424
import java.util.List;
25+
import java.util.stream.Stream;
2526
import java.util.stream.StreamSupport;
27+
import org.checkerframework.checker.nullness.qual.Nullable;
2628

2729
/** Utilities for AutoCloseable classes. */
2830
public final class AutoCloseables {
@@ -33,7 +35,8 @@ private AutoCloseables() {}
3335
* Returns a new {@link AutoCloseable} that calls {@link #close(Iterable)} on <code>autoCloseables
3436
* </code> when close is called.
3537
*/
36-
public static AutoCloseable all(final Collection<? extends AutoCloseable> autoCloseables) {
38+
public static AutoCloseable all(
39+
final @Nullable Collection<? extends @Nullable AutoCloseable> autoCloseables) {
3740
return new AutoCloseable() {
3841
@Override
3942
public void close() throws Exception {
@@ -48,7 +51,10 @@ public void close() throws Exception {
4851
* @param t the throwable to add suppressed exception to
4952
* @param autoCloseables the closeables to close
5053
*/
51-
public static void close(Throwable t, AutoCloseable... autoCloseables) {
54+
public static void close(Throwable t, @Nullable AutoCloseable... autoCloseables) {
55+
if (autoCloseables == null) {
56+
return;
57+
}
5258
close(t, Arrays.asList(autoCloseables));
5359
}
5460

@@ -58,7 +64,8 @@ public static void close(Throwable t, AutoCloseable... autoCloseables) {
5864
* @param t the throwable to add suppressed exception to
5965
* @param autoCloseables the closeables to close
6066
*/
61-
public static void close(Throwable t, Iterable<? extends AutoCloseable> autoCloseables) {
67+
public static void close(
68+
Throwable t, @Nullable Iterable<? extends @Nullable AutoCloseable> autoCloseables) {
6269
try {
6370
close(autoCloseables);
6471
} catch (Exception e) {
@@ -71,7 +78,10 @@ public static void close(Throwable t, Iterable<? extends AutoCloseable> autoClos
7178
*
7279
* @param autoCloseables the closeables to close
7380
*/
74-
public static void close(AutoCloseable... autoCloseables) throws Exception {
81+
public static void close(@Nullable AutoCloseable... autoCloseables) throws Exception {
82+
if (autoCloseables == null) {
83+
return;
84+
}
7585
close(Arrays.asList(autoCloseables));
7686
}
7787

@@ -80,7 +90,8 @@ public static void close(AutoCloseable... autoCloseables) throws Exception {
8090
*
8191
* @param ac the closeables to close
8292
*/
83-
public static void close(Iterable<? extends AutoCloseable> ac) throws Exception {
93+
public static void close(@Nullable Iterable<? extends @Nullable AutoCloseable> ac)
94+
throws Exception {
8495
// this method can be called on a single object if it implements Iterable<AutoCloseable>
8596
// like for example VectorContainer make sure we handle that properly
8697
if (ac == null) {
@@ -111,12 +122,17 @@ public static void close(Iterable<? extends AutoCloseable> ac) throws Exception
111122

112123
/** Calls {@link #close(Iterable)} on the flattened list of closeables. */
113124
@SafeVarargs
114-
public static void close(Iterable<? extends AutoCloseable>... closeables) throws Exception {
125+
public static void close(@Nullable Iterable<? extends @Nullable AutoCloseable>... closeables)
126+
throws Exception {
127+
if (closeables == null) {
128+
return;
129+
}
115130
close(flatten(closeables));
116131
}
117132

118133
@SafeVarargs
119-
private static Iterable<AutoCloseable> flatten(Iterable<? extends AutoCloseable>... closeables) {
134+
private static Iterable<AutoCloseable> flatten(
135+
Iterable<? extends @Nullable AutoCloseable>... closeables) {
120136
return new Iterable<AutoCloseable>() {
121137
// Cast from Iterable<? extends AutoCloseable> to Iterable<AutoCloseable> is safe in this
122138
// context
@@ -127,16 +143,18 @@ public Iterator<AutoCloseable> iterator() {
127143
return Arrays.stream(closeables)
128144
.flatMap(
129145
(Iterable<? extends AutoCloseable> i) ->
130-
StreamSupport.stream(
131-
((Iterable<AutoCloseable>) i).spliterator(), /* parallel= */ false))
146+
i == null
147+
? Stream.empty()
148+
: StreamSupport.stream(
149+
((Iterable<AutoCloseable>) i).spliterator(), /* parallel= */ false))
132150
.iterator();
133151
}
134152
};
135153
}
136154

137155
/** Converts <code>ac</code> to a {@link Iterable} filtering out any null values. */
138-
public static Iterable<AutoCloseable> iter(AutoCloseable... ac) {
139-
if (ac.length == 0) {
156+
public static Iterable<AutoCloseable> iter(@Nullable AutoCloseable... ac) {
157+
if (ac == null || ac.length == 0) {
140158
return Collections.emptyList();
141159
} else {
142160
final List<AutoCloseable> nonNullAc = new ArrayList<>();
@@ -153,10 +171,11 @@ public static Iterable<AutoCloseable> iter(AutoCloseable... ac) {
153171
public static class RollbackCloseable implements AutoCloseable {
154172

155173
private boolean commit = false;
156-
private List<AutoCloseable> closeables;
174+
private final List<AutoCloseable> closeables;
157175

158-
public RollbackCloseable(AutoCloseable... closeables) {
159-
this.closeables = new ArrayList<>(Arrays.asList(closeables));
176+
public RollbackCloseable(@Nullable AutoCloseable... closeables) {
177+
this.closeables =
178+
closeables == null ? new ArrayList<>() : new ArrayList<>(Arrays.asList(closeables));
160179
}
161180

162181
public <T extends AutoCloseable> T add(T t) {
@@ -165,12 +184,18 @@ public <T extends AutoCloseable> T add(T t) {
165184
}
166185

167186
/** Add all of <code>list</code> to the rollback list. */
168-
public void addAll(AutoCloseable... list) {
187+
public void addAll(@Nullable AutoCloseable... list) {
188+
if (list == null) {
189+
return;
190+
}
169191
closeables.addAll(Arrays.asList(list));
170192
}
171193

172194
/** Add all of <code>list</code> to the rollback list. */
173-
public void addAll(Iterable<? extends AutoCloseable> list) {
195+
public void addAll(@Nullable Iterable<? extends @Nullable AutoCloseable> list) {
196+
if (list == null) {
197+
return;
198+
}
174199
for (AutoCloseable ac : list) {
175200
closeables.add(ac);
176201
}
@@ -189,7 +214,7 @@ public void close() throws Exception {
189214
}
190215

191216
/** Creates an {@link RollbackCloseable} from the given closeables. */
192-
public static RollbackCloseable rollbackable(AutoCloseable... closeables) {
217+
public static RollbackCloseable rollbackable(@Nullable AutoCloseable... closeables) {
193218
return new RollbackCloseable(closeables);
194219
}
195220

@@ -203,7 +228,7 @@ public static RollbackCloseable rollbackable(AutoCloseable... closeables) {
203228
* @throws RuntimeException if an Exception occurs; the Exception is wrapped by the
204229
* RuntimeException
205230
*/
206-
public static void closeNoChecked(final AutoCloseable autoCloseable) {
231+
public static void closeNoChecked(final @Nullable AutoCloseable autoCloseable) {
207232
if (autoCloseable != null) {
208233
try {
209234
autoCloseable.close();

0 commit comments

Comments
 (0)