Skip to content

Commit 780e266

Browse files
authored
cleanup: remove dead exception-handling paths from closePdfResources (#472)
Both PDDocument.close() (verapdf) and StaticLayoutContainers .closeContrastRatioConsumer() swallow exceptions internally and have no throws clause. The accumulator pattern in closePdfResources() (capture into closeFailure, addSuppressed, conditional re-throw) and the matching try/catch in processFileWithResult's finally block were dead code in practice — neither path could fire under any of the currently-imported verapdf versions. Route both close calls through the existing clearCleanupStep helper so they share the same per-step isolation as the static-container resets. The method now declares no throws and the caller's finally block collapses to a single line. Behaviorally equivalent: cleanup is still always invoked, per-step isolation still prevents one failure from aborting the rest, processing exceptions still surface unchanged. Verified locally — file descriptor released after processFile returns (lsof shows neither the input PDF nor verapdf's temp PDF), 566 core tests pass. Closes #470
1 parent cb2b93c commit 780e266

1 file changed

Lines changed: 10 additions & 42 deletions

File tree

java/opendataloader-pdf-core/src/main/java/org/opendataloader/pdf/processors/DocumentProcessor.java

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,15 @@ public class DocumentProcessor {
7575
* - Clears static containers to remove lingering references
7676
* Should always be called in a finally block.
7777
*/
78-
private static void closePdfResources() throws Exception {
79-
Exception closeFailure = null;
80-
PDDocument document = StaticResources.getDocument();
81-
if (document != null) {
82-
try {
78+
private static void closePdfResources() {
79+
clearCleanupStep("PDDocument", () -> {
80+
PDDocument document = StaticResources.getDocument();
81+
if (document != null) {
8382
document.close();
84-
} catch (Exception e) {
85-
closeFailure = e;
86-
}
87-
}
88-
89-
try {
90-
StaticLayoutContainers.closeContrastRatioConsumer();
91-
} catch (Exception e) {
92-
if (closeFailure != null) {
93-
closeFailure.addSuppressed(e);
94-
} else {
95-
closeFailure = e;
9683
}
97-
}
84+
});
85+
clearCleanupStep("ContrastRatioConsumer", StaticLayoutContainers::closeContrastRatioConsumer);
9886

99-
// cleanup static containers
10087
clearCleanupStep("StaticResources", StaticResources::clear);
10188
clearCleanupStep("StaticContainers", () -> StaticContainers.updateContainers(null));
10289
clearCleanupStep(
@@ -107,10 +94,6 @@ private static void closePdfResources() throws Exception {
10794
clearCleanupStep("StaticStorages", StaticStorages::clearAllContainers);
10895
clearCleanupStep("StaticCoreContainers", StaticCoreContainers::clearAllContainers);
10996
clearCleanupStep("StaticXmpCoreContainers", StaticXmpCoreContainers::clearAllContainers);
110-
111-
if (closeFailure != null) {
112-
throw closeFailure;
113-
}
11497
}
11598

11699
/**
@@ -149,7 +132,6 @@ public static void processFile(String inputPdfName, Config config) throws IOExce
149132
* @throws IOException if unable to process the file
150133
*/
151134
public static ProcessingResult processFileWithResult(String inputPdfName, Config config) throws IOException {
152-
Throwable processingFailure = null;
153135
try {
154136
// Phase 1: Extract
155137
ExtractionResult extraction = extractContents(inputPdfName, config);
@@ -160,25 +142,11 @@ public static ProcessingResult processFileWithResult(String inputPdfName, Config
160142
long outputNs = System.nanoTime() - t0;
161143

162144
return new ProcessingResult(extraction.getHybridTimings(), extraction.getExtractionNs(), outputNs);
163-
} catch (IOException | RuntimeException | Error e) {
164-
processingFailure = e;
165-
throw e;
166145
} finally {
167-
// Ensures resources are always released, even if processing throws an exception
168-
try {
169-
closePdfResources();
170-
} catch (Exception closeException) {
171-
LOGGER.log(Level.WARNING, "Error during PDF resource cleanup", closeException);
172-
if (processingFailure != null) {
173-
processingFailure.addSuppressed(closeException);
174-
} else {
175-
if (closeException instanceof IOException) {
176-
throw (IOException) closeException;
177-
} else {
178-
throw new IOException("Failed to close PDF resources", closeException);
179-
}
180-
}
181-
}
146+
// Always release resources, even if processing threw. closePdfResources
147+
// logs and swallows per-step failures so cleanup cannot mask the original
148+
// processing exception.
149+
closePdfResources();
182150
}
183151
}
184152

0 commit comments

Comments
 (0)