Fix close unused xlsx sheet and workbook XML streams

[leehaut]

Purpose of the pull request

Fix InputStream leaks in XlsxSaxAnalyser when reading XLSX files.

Hidden sheets skipped via ignoreHiddenSheet, sheets that are opened but never matched, and workbook.xml streams were not always closed. Over many files this can hold zip/package entry streams longer than needed.

What's changed?

• Close the sheet InputStream immediately when the constructor skips a sheet (ctSheet == null), instead of dropping the handle on continue.
• Close every remaining stream in sheetMap from execute()'s finally block. Matched sheets are already closed by parseXmlSource; closing again is safe.
• Close xssfReader.getWorkbookData() with try-with-resources in analysisUse1904WindowDate and analysisCtSheetMap.
• Close failures on unused streams are logged and do not abort cleanup of other sheets.
• Keep a local matchedSheet instead of reassigning the loop variable.

No public API change. Read results are unchanged.

Test plan

• Added XlsxSaxAnalyserSheetStreamCloseTest:
  • reading only sheet 0 of a 3-sheet workbook closes unread sheet streams
  • a listener exception still closes remaining streams
  • ignoreHiddenSheet=true does not retain hidden-sheet streams and closes the visible ones after read
• Existing HiddenSheetsTest, XlsxSaxAnalyserReadOpcPackageTest, and DateWindowingTest passed
• mvn -pl fesod-sheet test -Dmaven.test.skip=false -Dtest=XlsxSaxAnalyserSheetStreamCloseTest,HiddenSheetsTest,XlsxSaxAnalyserReadOpcPackageTest,DateWindowingTest

Checklist

• I have read the Contributor Guide.
• I have written the necessary doc or comment.
• I have added the necessary unit tests and all cases have passed.

[skytin1004]

Hi @leehaut. The changes look good to me. Could you update the hidden-sheet test to also verify that the skipped sheet's stream is closed?

That test currently passes without the fix because it only checks that the hidden sheet is excluded from the sheet list.

[nkuprins]

FYI,

SimpleReadCacheSelector:readCache on 102 uses available() method directly on sharedStringsTablePackagePart.getInputStream(), so a new stream created by getInputStream() is not closed.

Remedy:

try (InputStream inputStream = sharedStringsTablePackagePart.getInputStream()) {
    size = inputStream.available();
} catch (IOException e) {
    log.warn("Unable to get file size, default used MapCache");
    return new MapCache();
}

This may relate to this PR, so you might consider fixing it here as well.

[leehaut]

Hi @leehaut. The changes look good to me. Could you update the hidden-sheet test to also verify that the skipped sheet's stream is closed?

That test currently passes without the fix because it only checks that the hidden sheet is excluded from the sheet list.

Thanks, that’s a fair point. The current assertions only prove the hidden sheet is omitted from sheetList/sheetMap, which already held before this fix. I’ll add a regression that records streams closed during construction and asserts the skipped hidden sheet stream is closed.

[leehaut]

FYI,

SimpleReadCacheSelector:readCache on 102 uses available() method directly on sharedStringsTablePackagePart.getInputStream(), so a new stream created by getInputStream() is not closed.

Remedy:

try (InputStream inputStream = sharedStringsTablePackagePart.getInputStream()) {
    size = inputStream.available();
} catch (IOException e) {
    log.warn("Unable to get file size, default used MapCache");
    return new MapCache();
}

This may relate to this PR, so you might consider fixing it here as well.

Thanks for catching that. I’ll close that SST InputStream with try-with-resources in this PR as well, since it is the same class of leak on the XLSX read path.

[skytin1004]

Thanks for the update, @leehaut.
The new test addresses my earlier concern. LGTM!