Skip to content

Commit 0092bcd

Browse files
ohadzeligerohadzeliger
authored
Implement a closeAll utility to close multiple closeables (#3470)
Tech debt from the ThrottlingIterator work. Close multiple closeables and capture all exceptions. Resolve #3465
1 parent ec143fc commit 0092bcd

File tree

5 files changed

+213
-20
lines changed

5 files changed

+213
-20
lines changed

‎fdb-extensions/src/main/java/com/apple/foundationdb/util/CloseException.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* CloseException.java
3+
*
4+
* This source file is part of the FoundationDB open source project
5+
*
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
21+
package com.apple.foundationdb.util;
22+
23+
/**
24+
* Exception thrown when the {@link CloseableUtils#closeAll} method catches an exception.
25+
* This exception will have the {@code cause} set to the first exception thrown during {@code closeAll} and any further
26+
* exception thrown will be added as {@code Suppressed}.
27+
*/
28+
@SuppressWarnings("serial")
29+
public class CloseException extends Exception {
30+
public CloseException(final Throwable cause) {
31+
super(cause);
32+
}
33+
}

‎fdb-extensions/src/main/java/com/apple/foundationdb/util/CloseableUtils.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* CloseableUtils.java
3+
*
4+
* This source file is part of the FoundationDB open source project
5+
*
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
21+
package com.apple.foundationdb.util;
22+
23+
import com.apple.foundationdb.annotation.API;
24+
25+
/**
26+
* Utility methods to help interact with {@link AutoCloseable} classes.
27+
**/
28+
public class CloseableUtils {
29+
/**
30+
* A utility to close multiple {@link AutoCloseable} objects, preserving all the caught exceptions.
31+
* The method would attempt to close all closeables in order, even if some failed.
32+
* Note that {@link CloseException} is used to wrap any exception thrown during the closing process. The reason for
33+
* that is the compiler fails to compile a {@link AutoCloseable#close()} implementation that throws a generic
34+
* {@link Exception} (due to {@link InterruptedException} issue) - We therefore have to catch and wrap all exceptions.
35+
* @param closeables the given sequence of {@link AutoCloseable}
36+
* @throws CloseException in case any exception was caught during the process. The first exception will be added
37+
* as a {@code cause}. In case more than one exception was caught, it will be added as Suppressed.
38+
*/
39+
@API(API.Status.INTERNAL)
40+
@SuppressWarnings("PMD.CloseResource")
41+
public static void closeAll(AutoCloseable... closeables) throws CloseException {
42+
CloseException accumulatedException = null;
43+
for (AutoCloseable closeable: closeables) {
44+
try {
45+
closeable.close();
46+
} catch (InterruptedException e) {
47+
Thread.currentThread().interrupt();
48+
if (accumulatedException == null) {
49+
accumulatedException = new CloseException(e);
50+
} else {
51+
accumulatedException.addSuppressed(e);
52+
}
53+
} catch (Exception e) {
54+
if (accumulatedException == null) {
55+
accumulatedException = new CloseException(e);
56+
} else {
57+
accumulatedException.addSuppressed(e);
58+
}
59+
}
60+
}
61+
if (accumulatedException != null) {
62+
throw accumulatedException;
63+
}
64+
}
65+
66+
private CloseableUtils() {
67+
// prevent constructor from being called
68+
}
69+
}

‎fdb-extensions/src/test/java/com/apple/foundationdb/util/CloseableUtilsTest.java

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* CloseableUtilsTest.java
3+
*
4+
* This source file is part of the FoundationDB open source project
5+
*
6+
* Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
21+
package com.apple.foundationdb.util;
22+
23+
import org.junit.jupiter.api.Assertions;
24+
import org.junit.jupiter.api.Test;
25+
26+
import static org.junit.jupiter.api.Assertions.assertThrows;
27+
28+
/**
29+
* Tests for the {@link CloseableUtils} class.
30+
*/
31+
public class CloseableUtilsTest {
32+
33+
@Test
34+
void closeAllNoIssue() throws Exception {
35+
SimpleCloseable c1 = new SimpleCloseable(false, null);
36+
SimpleCloseable c2 = new SimpleCloseable(false, null);
37+
SimpleCloseable c3 = new SimpleCloseable(false, null);
38+
39+
CloseableUtils.closeAll(c1, c2, c3);
40+
41+
Assertions.assertTrue(c1.isClosed());
42+
Assertions.assertTrue(c2.isClosed());
43+
Assertions.assertTrue(c3.isClosed());
44+
}
45+
46+
@Test
47+
void closeAllFailed() throws Exception {
48+
SimpleCloseable c1 = new SimpleCloseable(true, "c1");
49+
SimpleCloseable c2 = new SimpleCloseable(true, "c2");
50+
SimpleCloseable c3 = new SimpleCloseable(true, "c3");
51+
52+
final CloseException exception = assertThrows(CloseException.class, () -> CloseableUtils.closeAll(c1, c2, c3));
53+
54+
Assertions.assertEquals("c1", exception.getCause().getMessage());
55+
final Throwable[] suppressed = exception.getSuppressed();
56+
Assertions.assertEquals(2, suppressed.length);
57+
Assertions.assertEquals("c2", suppressed[0].getMessage());
58+
Assertions.assertEquals("c3", suppressed[1].getMessage());
59+
60+
Assertions.assertTrue(c1.isClosed());
61+
Assertions.assertTrue(c2.isClosed());
62+
Assertions.assertTrue(c3.isClosed());
63+
}
64+
65+
@Test
66+
void closeSomeFailed() throws Exception {
67+
SimpleCloseable c1 = new SimpleCloseable(true, "c1");
68+
SimpleCloseable c2 = new SimpleCloseable(false, null);
69+
SimpleCloseable c3 = new SimpleCloseable(true, "c3");
70+
71+
final CloseException exception = assertThrows(CloseException.class, () -> CloseableUtils.closeAll(c1, c2, c3));
72+
73+
Assertions.assertEquals("c1", exception.getCause().getMessage());
74+
final Throwable[] suppressed = exception.getSuppressed();
75+
Assertions.assertEquals(1, suppressed.length);
76+
Assertions.assertEquals("c3", suppressed[0].getMessage());
77+
78+
Assertions.assertTrue(c1.isClosed());
79+
Assertions.assertTrue(c2.isClosed());
80+
Assertions.assertTrue(c3.isClosed());
81+
}
82+
83+
private class SimpleCloseable implements AutoCloseable {
84+
private final boolean fail;
85+
private final String message;
86+
private boolean closed = false;
87+
88+
public SimpleCloseable(boolean fail, String message) {
89+
this.fail = fail;
90+
this.message = message;
91+
}
92+
93+
@Override
94+
public void close() {
95+
closed = true;
96+
if (fail) {
97+
throw new RuntimeException(message);
98+
}
99+
}
100+
101+
public boolean isClosed() {
102+
return closed;
103+
}
104+
}
105+
}

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/recordrepair/RecordRepair.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.apple.foundationdb.record.provider.foundationdb.runners.throttled.CursorFactory;
3131
import com.apple.foundationdb.record.provider.foundationdb.runners.throttled.ThrottledRetryingIterator;
3232
import com.apple.foundationdb.tuple.Tuple;
33+
import com.apple.foundationdb.util.CloseException;
3334
import org.slf4j.Logger;
3435
import org.slf4j.LoggerFactory;
3536

@@ -114,7 +115,7 @@ public static Builder builder(@Nonnull FDBDatabase database, final FDBRecordStor
114115
}
115116

116117
@Override
117-
public void close() {
118+
public void close() throws CloseException {
118119
throttledIterator.close();
119120
}
120121

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/runners/throttled/ThrottledRetryingIterator.java

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStore;
3434
import com.apple.foundationdb.record.provider.foundationdb.runners.FutureAutoClose;
3535
import com.apple.foundationdb.record.provider.foundationdb.runners.TransactionalRunner;
36+
import com.apple.foundationdb.util.CloseException;
37+
import com.apple.foundationdb.util.CloseableUtils;
3638
import com.google.common.annotations.VisibleForTesting;
3739
import org.slf4j.Logger;
3840
import org.slf4j.LoggerFactory;
@@ -149,30 +151,13 @@ public CompletableFuture<Void> iterateAll(final FDBRecordStore.Builder storeBuil
149151
}
150152

151153
@Override
152-
public void close() {
154+
public void close() throws CloseException {
153155
if (closed) {
154156
return;
155157
}
156158
closed = true;
157159
// Ensure we call both close() methods, capturing all exceptions
158-
RuntimeException caught = null;
159-
try {
160-
futureManager.close();
161-
} catch (RuntimeException e) {
162-
caught = e;
163-
}
164-
try {
165-
transactionalRunner.close();
166-
} catch (RuntimeException e) {
167-
if (caught != null) {
168-
caught.addSuppressed(e);
169-
} else {
170-
caught = e;
171-
}
172-
}
173-
if (caught != null) {
174-
throw caught;
175-
}
160+
CloseableUtils.closeAll(futureManager, transactionalRunner);
176161
}
177162

178163
/**

0 commit comments

Comments
 (0)