Skip to content

Commit c0d2086

Browse files
authored
fix: Upgrade 3PP versions and fix gRPC stream leak exposed by gRPC 1.80 (#168)
## Versioning restructuring Netty version management was previously split across two separate version entries (io-netty for netty-common at 4.2.x and netty-codec at 4.1.x). This is consolidated into a single `netty` version (4.1.132) applied uniformly via dependency constraints in java-base-conventions.gradle.kts. The soft-pinning rule in version-updates.gradle.kts now applies to all io.netty modules (not just netty-codec), since gRPC requires the 4.1.x line. Jackson versions were similarly consolidated from two separate entries into a single `jackson` version. JUnit library versions are now managed via the BOM instead of explicit per-artifact versions. The soft-pinning rule was split to handle JUnit Platform (1.x) separately from JUnit Jupiter (5.x). ## gRPC stream close fix gRPC 1.80 is stricter about channel shutdown: `channel.shutdown()` now properly waits for all active streams to complete before terminating. This exposed a pre-existing stream leak in the driver where closing a ResultSet did not cancel the underlying gRPC stream. The leak path was: ArrowStreamReader wraps ByteStringReadableByteChannel which wraps the gRPC iterator, but ByteStringReadableByteChannel.close() did not propagate close to the iterator, leaving orphaned gRPC streams on the channel. Fix: Introduce CloseableIterator<T> (Iterator + AutoCloseable) and use it throughout the stream chain. ByteStringReadableByteChannel now accepts a CloseableIterator and closes it when the channel is closed. SQLExceptionQueryResultIterator implements CloseableIterator and delegates close to the wrapped gRPC iterator. This ensures the full close chain works: StreamingResultSet.close() → ArrowStreamReader → ByteStringReadableByteChannel → gRPC stream cancel. JdbcDriverStubProvider.close() is also changed from graceful shutdown() to shutdownNow() as defense-in-depth, since Connection.close() means the caller is done and there is no need to wait for in-flight RPCs. ## Dependency upgrades - gRPC 1.78.0 → 1.80.0 - protobuf 4.33.4 → 4.34.1 - grpcmock 0.16.0 → 1.0.0 - jackson 2.21.0 → 2.21.2 - log4j 2.25.3 → 2.25.4 - JUnit BOM 5.14.2 → 5.14.3 - PostgreSQL JDBC 42.7.9 → 42.7.10 - scalatest 3.2.19 → 3.2.20 - shadow plugin 9.3.1 → 9.4.1 - buf plugin 0.10.3 → 0.11.0 - Netty 4.1.130 → 4.1.132
1 parent 18d9f38 commit c0d2086

23 files changed

Lines changed: 334 additions & 91 deletions

buildSrc/src/main/kotlin/java-base-conventions.gradle.kts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,39 @@ repositories {
1313
}
1414
}
1515

16+
val nettyVersion = extensions.getByType<VersionCatalogsExtension>()
17+
.named("libs")
18+
.findVersion("netty")
19+
.get()
20+
.requiredVersion
21+
22+
// Exclude grpc-netty-shaded (pulled in transitively by grpcmock) to prevent it from winning
23+
// gRPC provider discovery over our grpc-netty. The shaded variant has higher priority, and
24+
// mixing its older transport with our grpc-core version causes channels to never terminate.
25+
configurations.all {
26+
exclude(group = "io.grpc", module = "grpc-netty-shaded")
27+
}
28+
29+
dependencies {
30+
constraints {
31+
// gRPC and Arrow pull in Netty 4.1.x transitively. These constraints enforce a minimum
32+
// version to fix security vulnerabilities. Keep the version in gradle/libs.versions.toml.
33+
listOf(
34+
"io.netty:netty-buffer",
35+
"io.netty:netty-codec",
36+
"io.netty:netty-codec-http",
37+
"io.netty:netty-codec-http2",
38+
"io.netty:netty-common",
39+
"io.netty:netty-handler",
40+
"io.netty:netty-resolver",
41+
"io.netty:netty-transport",
42+
"io.netty:netty-transport-native-unix-common",
43+
).forEach { module ->
44+
implementation("$module:$nettyVersion")
45+
}
46+
}
47+
}
48+
1649
tasks.withType<Test>().configureEach {
1750
javaLauncher = javaToolchains.launcherFor {
1851
languageVersion = JavaLanguageVersion.of(8)

buildSrc/src/main/kotlin/version-updates.gradle.kts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@ versionCatalogUpdate {
1616
val regex = "^[0-9,.v-]+(-r)?$".toRegex()
1717

1818
// Special cases for libraries where we have to do soft pinning. We don't want to fully pin to still get the latest version for the allowed range.
19-
if (("org.junit" in it.candidate.group) && !candidateVersion.startsWith("5.")) {
20-
// This is soft pinned to 5.* (as 6 is not Java 8 compatible)
19+
if ("org.junit.platform" in it.candidate.group && !candidateVersion.startsWith("1.")) {
20+
// JUnit Platform uses 1.x versioning; pinned to 1.* (corresponding to JUnit Jupiter 5.x)
21+
false
22+
} else if ("org.junit" in it.candidate.group && !candidateVersion.startsWith("5.")) {
23+
// JUnit BOM and Jupiter are soft pinned to 5.* (as 6 is not Java 8 compatible)
2124
false
2225
} else if ("org.apache.spark" in it.candidate.group && !candidateVersion.startsWith("3.")) {
2326
// This is soft pinned to 3.* (as the driver is targetting in Spark 3)
@@ -28,9 +31,8 @@ versionCatalogUpdate {
2831
} else if ("org.mockito" in it.candidate.group && !candidateVersion.startsWith("4.")) {
2932
//This is soft pinned to 4.* (as 5 is not Java 8 compatible)
3033
false
31-
} else if ("io.netty" in it.candidate.group && "netty-codec" in it.candidate.module && !candidateVersion.startsWith("4.1.")) {
32-
// netty-codec is soft pinned to 4.1.* (as 4.2+ may have compatibility issues)
33-
// This restriction applies only to the netty-codec version entry, not other netty libraries
34+
} else if ("io.netty" in it.candidate.group && !candidateVersion.startsWith("4.1.")) {
35+
// Netty is soft pinned to 4.1.* as gRPC requires the 4.1.x line
3436
false
3537
} else {
3638
stableKeyword || regex.matches(candidateVersion)

gradle/libs.versions.toml

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,20 @@
44
apache-httpcomponents-httpclient = "5.6"
55
# @pin this version as it is the last Java 8 compatible version of Arrow
66
arrow = "17.0.0"
7-
com-fasterxml-jackson-core-jackson-databind = "2.21.0"
8-
com-fasterxml-jackson-module-jackson-module-scala = "2.21.0"
7+
jackson = "2.21.2"
98
# This has special version logic in `buildSrc/src/main/kotlin/version-updates.gradle.kts` to also pick the -jre version
109
com-google-guava-guava = "33.5.0-jre"
11-
grpc = "1.78.0"
12-
# This is only indirectly used by gRPC & Arrow but we provide an explicit version to fix security issues in the transitive dependencies
13-
io-netty = "4.2.9.Final"
10+
grpc = "1.80.0"
1411
javax-annotation-javax-annotation-api = "1.3.2"
1512
jjwt = "0.13.0"
16-
log4j-core = "2.25.3"
13+
log4j-core = "2.25.4"
1714
# This is soft pinned to 4.* (as 5 is not Java 8 compatible) in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
1815
mockito = "4.11.0"
1916
net-jodah-failsafe = "3.3.2"
20-
# This is soft pinned to 4.1.* (as 4.2+ may have compatibility issues) in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
21-
netty-codec = "4.1.130.Final"
17+
# Minimum Netty 4.1.x version, enforced via dependency constraints in `java-base-conventions.gradle.kts`
18+
# to fix security vulnerabilities in transitive dependencies pulled in by gRPC and Arrow.
19+
# This is soft pinned to 4.1.* in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
20+
netty = "4.1.132.Final"
2221
# This is soft pinned to 4.* (as we didn't invest in the upgrade yet) in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
2322
okhttp3 = "4.12.0"
2423
# @pin this version as the following versions have a breaking Cursor change, that - with the upcoming Avactica removal - we currently don't want to invest in
@@ -27,18 +26,18 @@ org-apache-commons-commons-lang3 = "3.20.0"
2726
# This is soft pinned to 3.* (as the driver is targetting in Spark 3) in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
2827
org-apache-spark = "3.5.8"
2928
org-assertj-assertj-core = "3.27.7"
30-
org-grpcmock-grpcmock-junit5 = "0.16.0"
29+
org-grpcmock-grpcmock-junit5 = "1.0.0"
30+
# JUnit BOM version — manages both Jupiter (5.x) and Platform (1.x) library versions.
3131
# This is soft pinned to 5.* (as 6 is not Java 8 compatible) in `buildSrc/src/main/kotlin/version-updates.gradle.kts`
32-
org-junit-bom = "5.14.2"
33-
org-junit-platform-junit-platform-launcher = "1.14.0"
34-
org-postgresql-pgjdbc = "42.7.9"
35-
org-scalatest = "3.2.19"
32+
org-junit-bom = "5.14.3"
33+
org-postgresql-pgjdbc = "42.7.10"
34+
org-scalatest = "3.2.20"
3635
org-scalatestplus-junit5 = "3.2.19.0"
37-
plugin-build-buf = "0.10.3"
36+
plugin-build-buf = "0.11.0"
3837
plugin-com-google-protobuf = "0.9.6"
39-
plugin-com-gradleup-shadow = "9.3.1"
38+
plugin-com-gradleup-shadow = "9.4.1"
4039
plugin-freefair-lombok = "9.2.0"
41-
protobuf = "4.33.4"
40+
protobuf = "4.34.1"
4241
slf4j = "2.0.17"
4342

4443
[libraries]
@@ -56,25 +55,25 @@ grpc-protoc = { module = "io.grpc:protoc-gen-grpc-java", version.ref = "grpc" }
5655
grpc-stub = { module = "io.grpc:grpc-stub", version.ref = "grpc" }
5756
grpcmock = { module = "org.grpcmock:grpcmock-junit5", version.ref = "org-grpcmock-grpcmock-junit5" }
5857
guava = { module = "com.google.guava:guava", version.ref = "com-google-guava-guava" }
59-
jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "com-fasterxml-jackson-core-jackson-databind" }
60-
jackson-module-scala = { module = "com.fasterxml.jackson.module:jackson-module-scala_2.13", version.ref = "com-fasterxml-jackson-module-jackson-module-scala" }
58+
jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson" }
59+
jackson-module-scala = { module = "com.fasterxml.jackson.module:jackson-module-scala_2.13", version.ref = "jackson" }
6160
javax-annotation-api = { module = "javax.annotation:javax.annotation-api", version.ref = "javax-annotation-javax-annotation-api" }
6261
jjwt-api = { module = "io.jsonwebtoken:jjwt-api", version.ref = "jjwt" }
6362
jjwt-impl = { module = "io.jsonwebtoken:jjwt-impl", version.ref = "jjwt" }
6463
jjwt-jackson = { module = "io.jsonwebtoken:jjwt-jackson", version.ref = "jjwt" }
65-
junit-bom = "org.junit:junit-bom:5.14.2"
66-
junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "org-junit-bom" }
67-
junit-jupiter-base = { module = "org.junit.jupiter:junit-jupiter", version.ref = "org-junit-bom" }
68-
junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "org-junit-bom" }
69-
junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "org-junit-bom" }
70-
junit-platform-engine = { module = "org.junit.platform:junit-platform-engine", version.ref = "org-junit-platform-junit-platform-launcher" }
71-
junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher", version.ref = "org-junit-platform-junit-platform-launcher" }
72-
junit-platform-launcher-test = { module = "org.junit.platform:junit-platform-launcher", version.ref = "org-junit-platform-junit-platform-launcher" }
64+
# Version managed by org-junit-bom; individual JUnit libraries below are versionless (BOM-managed)
65+
junit-bom = { module = "org.junit:junit-bom", version.ref = "org-junit-bom" }
66+
junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api" }
67+
junit-jupiter-base = { module = "org.junit.jupiter:junit-jupiter" }
68+
junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine" }
69+
junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params" }
70+
junit-platform-engine = { module = "org.junit.platform:junit-platform-engine" }
71+
junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher" }
7372
log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "log4j-core" }
7473
mockito-inline = { module = "org.mockito:mockito-inline", version.ref = "mockito" }
7574
mockito-junit-jupiter = { module = "org.mockito:mockito-junit-jupiter", version.ref = "mockito" }
76-
netty-codec = { module = "io.netty:netty-codec", version.ref = "netty-codec" }
77-
netty-common = { module = "io.netty:netty-common", version.ref = "io-netty" }
75+
netty-codec = { module = "io.netty:netty-codec", version.ref = "netty" }
76+
netty-common = { module = "io.netty:netty-common", version.ref = "netty" }
7877
okhttp3-client = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp3" }
7978
okhttp3-logging-interceptor = { module = "com.squareup.okhttp3:logging-interceptor", version.ref = "okhttp3" }
8079
okhttp3-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp3" }
@@ -112,7 +111,7 @@ mocking = [
112111
]
113112
scala-testing = [
114113
"junit-platform-engine",
115-
"junit-platform-launcher-test",
114+
"junit-platform-launcher",
116115
"scalatest",
117116
"scalatestplus-junit5",
118117
]

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/ByteStringReadableByteChannel.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,24 @@
55
package com.salesforce.datacloud.jdbc.core;
66

77
import com.google.protobuf.ByteString;
8+
import com.salesforce.datacloud.jdbc.protocol.CloseableIterator;
89
import java.io.IOException;
910
import java.nio.ByteBuffer;
1011
import java.nio.channels.ClosedChannelException;
1112
import java.nio.channels.ReadableByteChannel;
12-
import java.util.Iterator;
1313
import lombok.NonNull;
1414

1515
/**
1616
* A ReadableByteChannel that exposes an Iterator<ByteString> as a stream of bytes.
1717
* This class has a single responsibility: converting ByteString iterator to byte stream.
1818
*/
1919
public class ByteStringReadableByteChannel implements ReadableByteChannel {
20-
@NonNull private final Iterator<ByteString> iterator;
20+
@NonNull private final CloseableIterator<ByteString> iterator;
2121

2222
private boolean open = true;
2323
private ByteBuffer currentBuffer = null;
2424

25-
public ByteStringReadableByteChannel(@NonNull Iterator<ByteString> iterator) {
25+
public ByteStringReadableByteChannel(@NonNull CloseableIterator<ByteString> iterator) {
2626
this.iterator = iterator;
2727
}
2828

@@ -61,6 +61,13 @@ public boolean isOpen() {
6161
@Override
6262
public void close() throws IOException {
6363
open = false;
64+
try {
65+
iterator.close();
66+
} catch (IOException e) {
67+
throw e;
68+
} catch (Exception e) {
69+
throw new IOException("Failed to close underlying resource", e);
70+
}
6471
}
6572

6673
private static int transferToDestination(ByteBuffer source, ByteBuffer destination) {

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudStatement.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,12 @@ public int executeUpdate(String sql) throws SQLException {
195195
throw new SQLException(NOT_SUPPORTED_IN_DATACLOUD_QUERY, SqlErrorCodes.FEATURE_NOT_SUPPORTED);
196196
}
197197

198+
/**
199+
* Closes the statement and releases any resources associated with it. Might cancel the query
200+
* if it was not yet completed.
201+
*
202+
* @throws SQLException
203+
*/
198204
@Override
199205
public void close() throws SQLException {
200206
log.debug("Entering close");

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/JdbcDriverStubProvider.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,20 @@ public void close() {
5353
return;
5454
}
5555

56-
channel.shutdown();
56+
// Use shutdownNow() to cancel any in-flight RPCs. A JDBC Connection.close() means the
57+
// caller is done with the connection, so immediate cancellation is appropriate.
58+
// Note: With gRPC 1.80+, graceful shutdown() can hang if streams aren't fully closed,
59+
// e.g., when result sets are not explicitly closed before closing the connection.
60+
channel.shutdownNow();
5761

5862
try {
63+
// Brief wait after forceful shutdown. Any remaining transport cleanup happens
64+
// asynchronously in the background and will be handled by the JVM.
5965
if (!channel.awaitTermination(5, TimeUnit.SECONDS)) {
60-
log.warn("Channel did not terminate within 5 seconds, forcing shutdown");
61-
channel.shutdownNow();
66+
log.debug("Channel transport still shutting down in background after shutdownNow");
6267
}
6368
} catch (InterruptedException e) {
64-
log.warn("Channel shutdown interrupted, forcing immediate termination", e);
65-
channel.shutdownNow();
69+
log.warn("Channel shutdown interrupted", e);
6670
Thread.currentThread().interrupt();
6771
}
6872
}

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/SQLExceptionQueryResultIterator.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
package com.salesforce.datacloud.jdbc.core;
66

77
import com.salesforce.datacloud.jdbc.exception.QueryExceptionHandler;
8+
import com.salesforce.datacloud.jdbc.protocol.CloseableIterator;
89
import com.salesforce.datacloud.jdbc.protocol.QueryResultArrowStream;
910
import io.grpc.StatusRuntimeException;
10-
import java.util.Iterator;
1111
import lombok.AccessLevel;
1212
import lombok.AllArgsConstructor;
1313
import lombok.SneakyThrows;
@@ -26,12 +26,17 @@
2626
* @see QueryExceptionHandler
2727
*/
2828
@AllArgsConstructor(access = AccessLevel.PRIVATE)
29-
class SQLExceptionQueryResultIterator implements Iterator<QueryResult> {
30-
Iterator<QueryResult> grpcIterator;
29+
class SQLExceptionQueryResultIterator implements CloseableIterator<QueryResult> {
30+
CloseableIterator<QueryResult> resultIterator;
3131
boolean includeCustomerDetail;
3232
String queryId;
3333
String sql;
3434

35+
@Override
36+
public void close() throws Exception {
37+
resultIterator.close();
38+
}
39+
3540
/**
3641
* Creates an {@link ArrowStreamReader} that wraps the given iterator with SQL exception handling.
3742
* <p>
@@ -47,7 +52,7 @@ class SQLExceptionQueryResultIterator implements Iterator<QueryResult> {
4752
* @return an {@link ArrowStreamReader} that converts gRPC exceptions to SQL exceptions
4853
*/
4954
public static ArrowStreamReader createSqlExceptionArrowStreamReader(
50-
Iterator<QueryResult> resultIterator, boolean includeCustomerDetail, String queryId, String sql) {
55+
CloseableIterator<QueryResult> resultIterator, boolean includeCustomerDetail, String queryId, String sql) {
5156
val throwingSqlExceptionIterator =
5257
new SQLExceptionQueryResultIterator(resultIterator, includeCustomerDetail, queryId, sql);
5358
return QueryResultArrowStream.toArrowStreamReader(throwingSqlExceptionIterator);
@@ -67,7 +72,7 @@ public static ArrowStreamReader createSqlExceptionArrowStreamReader(
6772
@Override
6873
public boolean hasNext() {
6974
try {
70-
return grpcIterator.hasNext();
75+
return resultIterator.hasNext();
7176
} catch (StatusRuntimeException ex) {
7277
throw QueryExceptionHandler.createException(includeCustomerDetail, sql, queryId, ex);
7378
}
@@ -88,7 +93,7 @@ public boolean hasNext() {
8893
@Override
8994
public QueryResult next() {
9095
try {
91-
return grpcIterator.next();
96+
return resultIterator.next();
9297
} catch (StatusRuntimeException ex) {
9398
throw QueryExceptionHandler.createException(includeCustomerDetail, queryId, sql, ex);
9499
}

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/StreamingResultSet.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66

77
import static com.salesforce.datacloud.jdbc.util.ArrowUtils.toColumnMetaData;
88

9-
import com.salesforce.datacloud.jdbc.util.ThrowingJdbcSupplier;
10-
import com.salesforce.datacloud.query.v3.QueryStatus;
119
import java.io.IOException;
1210
import java.sql.ResultSet;
1311
import java.sql.ResultSetMetaData;
@@ -30,7 +28,6 @@ public class StreamingResultSet extends AvaticaResultSet implements DataCloudRes
3028
private final String queryId;
3129

3230
private final ArrowStreamReaderCursor cursor;
33-
ThrowingJdbcSupplier<QueryStatus> getQueryStatus;
3431
private final ColumnNameResolver columnNameResolver;
3532

3633
private StreamingResultSet(
@@ -68,6 +65,15 @@ public static StreamingResultSet of(ArrowStreamReader resultStream, String query
6865
}
6966
}
7067

68+
@Override
69+
public void close() {
70+
try {
71+
cursor.close();
72+
} finally {
73+
super.close();
74+
}
75+
}
76+
7177
@Override
7278
public int getType() {
7379
return ResultSet.TYPE_FORWARD_ONLY;
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* This file is part of https://github.com/forcedotcom/datacloud-jdbc which is released under the
3+
* Apache 2.0 license. See https://github.com/forcedotcom/datacloud-jdbc/blob/main/LICENSE.txt
4+
*/
5+
package com.salesforce.datacloud.jdbc.protocol;
6+
7+
import java.util.Iterator;
8+
9+
public interface CloseableIterator<T> extends Iterator<T>, AutoCloseable {
10+
11+
/**
12+
* Wraps a plain iterator as a CloseableIterator with a no-op close.
13+
*/
14+
static <T> CloseableIterator<T> of(Iterator<T> iterator) {
15+
return new CloseableIterator<T>() {
16+
@Override
17+
public boolean hasNext() {
18+
return iterator.hasNext();
19+
}
20+
21+
@Override
22+
public T next() {
23+
return iterator.next();
24+
}
25+
26+
@Override
27+
public void close() {}
28+
};
29+
}
30+
}

0 commit comments

Comments
 (0)