fix: address CodeQL resource lifetime warnings

[FrankChen021]

What changed

• Close test-owned input, output, archive, and JDBC resources with try-with-resources.
• Centralize repeated stream ownership where a downstream component closes the stream.
• Replace the obsolete LGTM annotation and document the few intentional lifecycle-owned resources with precise CodeQL suppressions.

Why

The current CodeQL snapshot reports 117 resource-lifetime warnings:

• 88 java/input-resource-leak
• 16 java/output-resource-leak
• 13 java/database-resource-leak

Most findings were real test cleanup gaps. The remaining ownership cases outlive the creating method by design and are closed by an enclosing Closer, connection, client teardown, or downstream copy operation.

Impact

This makes resource ownership explicit without changing production behavior. Test cleanup is deterministic, and the intentional long-lived cases are documented at the exact alert sites.

Root cause

Several tests relied on wrapper or fixture teardown to close nested resources, which CodeQL could not consistently prove. A small number of production and lifecycle tests intentionally transfer ownership beyond the creating method.

Checks

• 181 focused tests passed
• 2 tests skipped
• git diff --check

Created by GPT-5.6-Sol.

[Copilot]

Pull request overview

This PR reduces CodeQL resource-lifetime findings by making resource ownership explicit (primarily in tests) via try-with-resources, and by documenting a few intentional ownership transfers/leaks with targeted CodeQL suppressions. It keeps production behavior unchanged while improving determinism and clarity around cleanup.

Changes:

• Updated many tests to deterministically close JDBC, streams, and archive resources using try-with-resources.
• Centralized/annotated a few intentional ownership-transfer cases (e.g., resources closed by a Closer or by FileUtils.copyLarge).
• Replaced obsolete LGTM suppression with codeql[...] suppressions and added explanatory comments at the alert sites.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java	Closes JDBC resources in loops; adds targeted suppressions for intentionally-open statements/connections.
server/src/test/java/org/apache/druid/server/QueryResourceTest.java	Ensures request input streams are closed in exception-path tests.
server/src/test/java/org/apache/druid/server/initialization/JettyTest.java	Closes HTTP response input streams (gzip and non-gzip) deterministically.
server/src/test/java/org/apache/druid/rpc/RequestBuilderTest.java	Closes request content stream wrappers after assertions.
server/src/test/java/org/apache/druid/metadata/input/SqlEntityTest.java	Closes CleanableFile and file input streams when reading query results.
processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java	Closes FileInputStream when delegating to createIndex(InputStream, ...).
processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java	Expands try-with-resources coverage for compression/decompression stream chains.
processing/src/test/java/org/apache/druid/data/input/impl/RetryingInputStreamTest.java	Closes RetryingInputStream instances via try-with-resources in tests.
processing/src/main/java/org/apache/druid/segment/file/SegmentFileBuilderV10.java	Documents Closer-owned output stream lifetime with CodeQL suppression.
processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java	Replaces LGTM suppression with CodeQL suppression for Closer-owned stream.
indexing-service/src/test/java/org/apache/druid/metadata/SQLMetadataStorageActionHandlerTest.java	Closes JDBC Statement/ResultSet used for metadata query.
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HttpShuffleClientTest.java	Centralizes response stream creation and documents downstream-closed ownership with CodeQL suppression.
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java	Closes readers around streamed task log/report/status content.
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java	Closes chained gzip output streams explicitly.
extensions-core/druid-bloom-filter/src/test/java/org/apache/druid/query/filter/BloomKFilterTest.java	Introduces helper to close ByteBufferInputStream when deserializing.
extensions-contrib/time-min-max/src/test/java/org/apache/druid/query/aggregation/TimestampGroupByAggregationTest.java	Closes ZipFile and entry InputStream during ingestion/query setup.
extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java	Closes readers and config input streams loaded from classpath resources.
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java	Closes readers around streamed OSS task log/report content.
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentPullerTest.java	Ensures object content streams are closed in OSS segment puller tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java:82

• getResourceAsStream("white-rabbit.txt") can legally return null; new InputStreamReader(stream, …) would then throw a NullPointerException that bypasses the catch (IOException) block and produces a confusing class-initialization failure. Consider requiring a non-null resource stream with an explicit message.

    try (
        final InputStream stream = CompressionUtilsTest.class.getClassLoader().getResourceAsStream("white-rabbit.txt");
        final InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8);
        final Scanner scanner = new Scanner(reader).useDelimiter(Pattern.quote(System.lineSeparator()))
    ) {

indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HttpShuffleClientTest.java:206

• openSegmentFile() currently returns ListenableFuture<Object>, which loses type safety and makes it less clear that the future value is an InputStream. Returning ListenableFuture<InputStream> (without the explicit <Object> on immediateFuture) keeps intent and compile-time checking.

  private ListenableFuture<Object> openSegmentFile() throws FileNotFoundException
  {
    // Ownership passes through HttpShuffleClient to FileUtils.copyLarge, which closes the response stream.
    // codeql[java/input-resource-leak]
    return Futures.<Object>immediateFuture(new FileInputStream(segmentFile));

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java:1239

• exec.shutdown() and server.close() are executed only on the success path. If an assertion (or any exception) occurs in the query/assertion block, the server and executor can be left running and may impact subsequent tests. Use an outer try/finally and add any close failures as suppressed so they don’t mask the original test failure.

    try (final Connection smallFrameClient = server.getUserConnection();
         final PreparedStatement statement = smallFrameClient.prepareStatement("SELECT dim1 FROM druid.foo")) {
      // use a prepared statement because Avatica currently ignores fetchSize on the initial fetch of a Statement
      // set a fetch size below the minimum configured threshold
      statement.setFetchSize(2);

sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java:1179

• exec.shutdown() and server.close() are executed only on the success path. If an assertion (or any exception) occurs inside the try block, the Jetty server and executor can be left running, which can interfere with later tests. Wrap the body in an outer try/finally and ensure close exceptions do not mask the original test failure (add them as suppressed).

This issue also appears on line 1235 of the same file.

    ServerWrapper server = new ServerWrapper(smallFrameDruidMeta);
    try (final Connection smallFrameClient = server.getUserConnection();
         final Statement statement = smallFrameClient.createStatement();
         final ResultSet resultSet = statement.executeQuery("SELECT dim1 FROM druid.foo")) {
      final List<Map<String, Object>> rows = getRows(resultSet);

processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java:874

• The try-with-resources block declares both fileOutputStream and a FilterOutputStream that wraps it as resources. Closing outputStream already closes fileOutputStream, so closing both is redundant and can lead to confusing suppressed close exceptions if anything goes wrong during cleanup. Keep only the outer stream as the resource.

    try (
        final InputStream inputStream = new FileInputStream(gzFile);
        final OutputStream fileOutputStream = new FileOutputStream(testFile)
        {
          @Override

[FrankChen021]

Addressed the latest Copilot findings in commit 39c3a1d. ServerWrapper is now AutoCloseable, the max-frame, min-frame, and async Avatica tests close it on every exit path, their executors shut down in finally blocks, and the redundant nested CompressionUtils output-stream resource was removed. Targeted tests passed (3 Avatica tests and 1 CompressionUtils test), as did full processing/sql validation with Checkstyle and PMD.

[FrankChen021]

Also addressed the two earlier suppressed Copilot suggestions in commit e04124a: the white-rabbit fixture now has an explicit missing-resource diagnostic, and the HttpShuffleClient mock is typed through InputStreamResponseHandler so openSegmentFile returns ListenableFuture instead of ListenableFuture. CompressionUtilsTest (1 targeted test) and HttpShuffleClientTest (5 tests) passed; full processing/indexing-service validation with Checkstyle and PMD also passed.

[FrankChen021]

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 19 of 19 changed files.

---

This is an automated review by Codex GPT-5.6-Sol