Skip to content

Commit 1855308

Browse files
authored
fix: close pending zero-copy responses when Storage#close is called (#2696)
Update gRPC based Storage instances to close out in progress reads when zero-copy is used and Storage#close() is called.
1 parent d91ad84 commit 1855308

File tree

2 files changed

+41
-28
lines changed

2 files changed

+41
-28
lines changed

google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java

+35-27
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
import com.google.storage.v2.StorageSettings;
7171
import com.google.storage.v2.stub.GrpcStorageCallableFactory;
7272
import com.google.storage.v2.stub.GrpcStorageStub;
73-
import com.google.storage.v2.stub.StorageStub;
7473
import com.google.storage.v2.stub.StorageStubSettings;
7574
import io.grpc.ClientInterceptor;
7675
import io.grpc.Detachable;
@@ -89,9 +88,9 @@
8988
import java.time.Clock;
9089
import java.util.ArrayList;
9190
import java.util.Arrays;
92-
import java.util.Collection;
9391
import java.util.Collections;
9492
import java.util.IdentityHashMap;
93+
import java.util.Iterator;
9594
import java.util.List;
9695
import java.util.Locale;
9796
import java.util.Map;
@@ -908,9 +907,27 @@ private Object readResolve() {
908907

909908
private static final class InternalStorageClient extends StorageClient {
910909

911-
private InternalStorageClient(StorageStub stub) {
910+
private InternalStorageClient(InternalZeroCopyGrpcStorageStub stub) {
912911
super(stub);
913912
}
913+
914+
@Override
915+
public void shutdownNow() {
916+
try {
917+
// GrpcStorageStub#close() is final and we can't override it
918+
// instead hook in here to close out the zero-copy marshaller
919+
getStub().getObjectMediaResponseMarshaller.close();
920+
} catch (IOException e) {
921+
throw new RuntimeException(e);
922+
} finally {
923+
super.shutdownNow();
924+
}
925+
}
926+
927+
@Override
928+
public InternalZeroCopyGrpcStorageStub getStub() {
929+
return (InternalZeroCopyGrpcStorageStub) super.getStub();
930+
}
914931
}
915932

916933
private static final class InternalZeroCopyGrpcStorageStub extends GrpcStorageStub
@@ -1071,30 +1088,21 @@ public void close() throws IOException {
10711088
* them all as suppressed exceptions on the first occurrence.
10721089
*/
10731090
@VisibleForTesting
1074-
static void closeAllStreams(Collection<InputStream> inputStreams) throws IOException {
1075-
IOException ioException =
1076-
inputStreams.stream()
1077-
.map(
1078-
stream -> {
1079-
try {
1080-
stream.close();
1081-
return null;
1082-
} catch (IOException e) {
1083-
return e;
1084-
}
1085-
})
1086-
.filter(Objects::nonNull)
1087-
.reduce(
1088-
null,
1089-
(l, r) -> {
1090-
if (l != null) {
1091-
l.addSuppressed(r);
1092-
return l;
1093-
} else {
1094-
return r;
1095-
}
1096-
},
1097-
(l, r) -> l);
1091+
static void closeAllStreams(Iterable<InputStream> inputStreams) throws IOException {
1092+
Iterator<InputStream> iterator = inputStreams.iterator();
1093+
IOException ioException = null;
1094+
while (iterator.hasNext()) {
1095+
InputStream next = iterator.next();
1096+
try {
1097+
next.close();
1098+
} catch (IOException e) {
1099+
if (ioException == null) {
1100+
ioException = e;
1101+
} else {
1102+
ioException.addSuppressed(e);
1103+
}
1104+
}
1105+
}
10981106

10991107
if (ioException != null) {
11001108
throw ioException;

google-cloud-storage/src/main/java/com/google/cloud/storage/ResponseContentLifecycleManager.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,15 @@
1616
package com.google.cloud.storage;
1717

1818
import com.google.storage.v2.ReadObjectResponse;
19+
import java.io.Closeable;
20+
import java.io.IOException;
1921

20-
interface ResponseContentLifecycleManager {
22+
interface ResponseContentLifecycleManager extends Closeable {
2123
ResponseContentLifecycleHandle get(ReadObjectResponse response);
2224

25+
@Override
26+
default void close() throws IOException {}
27+
2328
static ResponseContentLifecycleManager noop() {
2429
return response ->
2530
new ResponseContentLifecycleHandle(

0 commit comments

Comments
 (0)