API: Close encryption manager after FileIO close failure

[hkwi]

Summary

• close a closeable encryption manager even when the delegate FileIO fails to close
• retain the delegate failure as the primary exception and suppress a subsequent encryption-manager close failure
• add regression coverage for both failure paths

Motivation

EncryptingFileIO.close() currently closes its delegate before closing a closeable encryption
manager. If the delegate throws, the encryption manager is never closed. This can leak resources
owned by the encryption manager, such as a KMS client.

The updated implementation uses try-with-resources to preserve the existing close order while
ensuring both resources are closed with standard suppressed-exception behavior.

This was identified while reviewing the KMS client lifecycle for Apache Polaris:

apache/polaris#5060 (comment)

Testing

• ./gradlew :iceberg-api:test --tests org.apache.iceberg.encryption.TestEncryptingFileIO --no-daemon --console=plain
• ./gradlew :iceberg-api:spotlessCheck --no-daemon --console=plain
• ./gradlew :iceberg-api:check --no-daemon --console=plain

---

AI Disclosure

• Model: GPT-5
• Platform/Tool: Codex
• Human Oversight: partially reviewed
• Prompt Summary: Fix EncryptingFileIO.close() so all owned resources are closed safely when the delegate close fails, and add regression tests.

[gaborkaszab]

With this implementation we write io.close() twice. Maybe running it first as before but in a try-finally block results cleaner code. What I have in mind:

try {
      io.close();
    } finally {
      if (em instanceof Closeable closeableManager) {
        try {
          closeableManager.close();
        } catch (IOException e) {
          throw new UncheckedIOException("Failed to close encryption manager", e);
        }
      }
    }

WDYT?

[hkwi]

Good point — I agree that keeping io.close() in one place would make this cleaner. How about using a nullable try-with-resources resource instead?

@Override
public void close() {
  try (Closeable closeableManager =
      em instanceof Closeable ? (Closeable) em : null) {
    io.close();
  } catch (IOException e) {
    throw new UncheckedIOException("Failed to close encryption manager", e);
  }
}

This removes the duplication while preserving try-with-resources suppression semantics. If both io.close() and closeableManager.close() fail, the FileIO failure remains the primary exception and the manager failure is added as suppressed. With the proposed finally, the UncheckedIOException from the manager would replace the FileIO failure. WDYT?

[gaborkaszab]

Seems good to me. Thanks!