You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by fa...@apache.org on 2026/09/18 09:27:34 UTC
(poi) branch trunk updated: Cut the heap the integration tests need and fix their heap settings (#1277)
This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new 187b58519d Cut the heap the integration tests need and fix their heap settings (#1277)
187b58519d is described below
commit 187b58519db4f32700f68ac5cf1151b87d23703d
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Fri Sep 18 10:27:29 2026 +0100
Cut the heap the integration tests need and fix their heap settings (#1277)
* Cut the heap the integration tests need and fix their heap settings
The gradle CI job (GitHub Actions) never set CI_BUILD, so poi-integration
ran with the root 2g heap and one test thread per processor; the 3g/2-thread
config in poi-integration/build.gradle only applied on Jenkins. Under that
heap TestAllFiles intermittently stalls in "Retried waiting for GCLocker too
often" and then kills the JUnit engine with an OutOfMemoryError (seen on
the Java 21 job for #1276). Apply the 3g heap unconditionally, trigger the
parallelism cap on CI=true as well, and raise the ant heap to match.
Also reduce what the tests actually hold on the heap:
- IOUtils.toByteArray: when the length is known and fits the initial
allocation, read straight into the result array instead of through a
ByteArrayOutputStream plus toByteArray() copy. Halves the transient
memory of every sized read (zip entries up to 2MB, record payloads).
- TestAllFiles: lower the temp-file threshold from 16MB to 2MB. Since
76d93fcc0f (#1236) the threshold also bounds how much of an unknown-size
entry is buffered on the heap before spilling; the workbooks POI writes
to a stream (which SpreadsheetHandler re-reads for every spreadsheet) use
data descriptors, so their parts had moved from temp files onto the heap.
- SpreadsheetHandler: write the discarded first copy to NullOutputStream
and drop the serialized bytes before processing the re-read workbook.
- XSSFFileHandler, OPCFileHandler, XSSFBFileHandler: close the OPCPackages
opened from streams; each kept every decompressed part alive until GC.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
* Read the sized fast path in the same 4096-byte chunks as before
The zip-bomb ratio check runs per read, so reading a whole entry in one
call changed the byte counts in the 'Zip bomb detected!' message that
poi-integration-exceptions.csv expects for the poc-xmlbomb files.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <no...@anthropic.com>
---
build.xml | 2 +-
poi-integration/build.gradle | 21 ++++++++++++++-------
.../java/org/apache/poi/stress/OPCFileHandler.java | 7 ++++++-
.../org/apache/poi/stress/SpreadsheetHandler.java | 17 ++++++++++-------
.../java/org/apache/poi/stress/TestAllFiles.java | 19 ++++++++++++-------
.../org/apache/poi/stress/XSSFBFileHandler.java | 4 +++-
.../org/apache/poi/stress/XSSFFileHandler.java | 6 +++++-
poi/src/main/java/org/apache/poi/util/IOUtils.java | 22 ++++++++++++++++++++++
8 files changed, 73 insertions(+), 25 deletions(-)
diff --git a/build.xml b/build.xml
index 7619cd5add..603ac3d343 100644
--- a/build.xml
+++ b/build.xml
@@ -1837,7 +1837,7 @@ under the License.
<pathelement path="lib/ooxml-tests"/>
</path>
- <poiunit failureproperty="integration.test.failed" heap="2048" showoutput="true" jacocodest="build/jacoco-integration.exec"
+ <poiunit failureproperty="integration.test.failed" heap="3072" showoutput="true" jacocodest="build/jacoco-integration.exec"
modulepath-ref="test-integration.modules" module1="stress" outputDir="${integration.reports.test}">
<zipfileset src="build/dist/maven/poi-integration/poi-integration-${version.id}.jar">
<include name="**/${testpattern}.class"/>
diff --git a/poi-integration/build.gradle b/poi-integration/build.gradle
index 375b83da80..02b2c0d58b 100644
--- a/poi-integration/build.gradle
+++ b/poi-integration/build.gradle
@@ -144,18 +144,25 @@ test {
dependsOn { testJar }
- if (Boolean.valueOf(System.getenv("CI_BUILD"))) {
- // On CI, cap the parallelism and give the test JVM more heap: the default dynamic
- // strategy runs one thread per processor, and several tests materialising large
- // XSSF workbooks concurrently (the XSSF additional tests load each workbook twice
- // via ExcelComparator) drove the 2GB test JVM into OutOfMemoryError on the Jenkins
- // nodes, with tests slowing down progressively as the heap filled.
+ // TestAllFiles materialises several large workbooks at once (the XSSF handlers hold every
+ // decompressed part of a stream-opened workbook on the heap, and load each file two or
+ // three times over), so it needs more heap than the 2g the root build.gradle gives the
+ // other modules: under heap pressure the JVM first stalls in "Retried waiting for GCLocker
+ // too often" and then kills the JUnit engine with an OutOfMemoryError.
+ maxHeapSize = '3g'
+
+ // CI_BUILD is what the Jenkins nodes set; GitHub Actions sets CI=true.
+ if (Boolean.valueOf(System.getenv("CI_BUILD")) || Boolean.valueOf(System.getenv("CI"))) {
+ // On CI, also cap the parallelism: the default dynamic strategy runs one thread per
+ // processor, and several tests materialising large XSSF workbooks concurrently (the
+ // XSSF additional tests load each workbook twice via ExcelComparator) drove the test
+ // JVM into OutOfMemoryError on the Jenkins nodes, with tests slowing down
+ // progressively as the heap filled.
// Note: these systemProperties deliberately override the strictly-serial CI
// settings from the root build.gradle for this module.
systemProperties['junit.jupiter.execution.parallel.enabled'] = 'true'
systemProperties['junit.jupiter.execution.parallel.config.strategy'] = 'fixed'
systemProperties['junit.jupiter.execution.parallel.config.fixed.parallelism'] = '2'
- maxHeapSize = '3g'
} else {
systemProperties['junit.jupiter.execution.parallel.enabled'] = 'true'
}
diff --git a/poi-integration/src/test/java/org/apache/poi/stress/OPCFileHandler.java b/poi-integration/src/test/java/org/apache/poi/stress/OPCFileHandler.java
index 68234bc030..04a49ae5f3 100644
--- a/poi-integration/src/test/java/org/apache/poi/stress/OPCFileHandler.java
+++ b/poi-integration/src/test/java/org/apache/poi/stress/OPCFileHandler.java
@@ -26,6 +26,7 @@ import java.io.PushbackInputStream;
import java.nio.file.Files;
import java.util.Set;
+import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
import org.apache.poi.openxml4j.opc.ContentTypes;
import org.apache.poi.openxml4j.opc.OPCPackage;
import org.apache.poi.openxml4j.opc.PackagePart;
@@ -44,8 +45,12 @@ public class OPCFileHandler extends AbstractFileHandler {
if (StressTestUtils.excludeFile(path, EXPECTED_FAILURES)) return;
- OPCPackage p = OPCPackage.open(stream);
+ try (OPCPackage p = OPCPackage.open(stream)) {
+ checkParts(p);
+ }
+ }
+ private static void checkParts(OPCPackage p) throws InvalidFormatException {
for (PackagePart part : p.getParts()) {
if (part.getPartName().toString().equals("/docProps/core.xml")) {
assertEquals(ContentTypes.CORE_PROPERTIES_PART, part.getContentType());
diff --git a/poi-integration/src/test/java/org/apache/poi/stress/SpreadsheetHandler.java b/poi-integration/src/test/java/org/apache/poi/stress/SpreadsheetHandler.java
index 40f103b523..4f4ef33bcc 100644
--- a/poi-integration/src/test/java/org/apache/poi/stress/SpreadsheetHandler.java
+++ b/poi-integration/src/test/java/org/apache/poi/stress/SpreadsheetHandler.java
@@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.IOException;
+import org.apache.commons.io.output.NullOutputStream;
import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
import org.apache.poi.ss.extractor.EmbeddedData;
import org.apache.poi.ss.extractor.EmbeddedExtractor;
@@ -39,17 +40,19 @@ public abstract class SpreadsheetHandler extends AbstractFileHandler {
extractEmbedded(wb);
- // write out the file
- writeToArray(wb);
+ // write out the file - the output is not needed, so do not buffer it on the heap
+ wb.write(NullOutputStream.INSTANCE);
// access some more content (we had cases where writing corrupts the data in memory)
readContent(wb);
- // write once more
- UnsynchronizedByteArrayOutputStream out = writeToArray(wb);
-
- // read in the written file
- Workbook read = WorkbookFactory.create(out.toInputStream());
+ // write once more and read in the written file. The serialized bytes are only needed
+ // to open the second workbook, so keep them out of scope while it is processed - the
+ // second workbook holds its own (decompressed) copy of the parts.
+ final Workbook read;
+ try (UnsynchronizedByteArrayOutputStream out = writeToArray(wb)) {
+ read = WorkbookFactory.create(out.toInputStream());
+ }
assertNotNull(read);
diff --git a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java
index 9e178ffbd0..3a42f4ffcc 100644
--- a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java
+++ b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java
@@ -83,13 +83,18 @@ public class TestAllFiles {
ROOT_DIR = dir;
- // Buffer large (or unknown-size) zip entries to temp files instead of holding them in
- // memory. Some test files inflate to nearly the 100MB per-entry limit (e.g.
- // spreadsheet/deep-data.xlsx has an entry that inflates to ~92MB while staying just
- // above the minimum inflate ratio); materialising those on the heap on every open,
- // with tests running in parallel in a 2GB JVM, drives the CI builds into
- // OutOfMemoryError.
- ZipInputStreamZipEntrySource.setThresholdBytesForTempFiles(16_000_000);
+ // Buffer large zip entries to temp files instead of holding them in memory. Some test
+ // files inflate to nearly the 100MB per-entry limit (e.g. spreadsheet/deep-data.xlsx
+ // has an entry that inflates to ~92MB while staying just above the minimum inflate
+ // ratio); materialising those on the heap on every open, with tests running in
+ // parallel, drives the CI builds into OutOfMemoryError.
+ // The threshold is deliberately low: a stream-opened workbook keeps every part on the
+ // heap until it is closed, and the handlers open each file several times over (the
+ // original, the re-read of the written copy, the extractors) with a few tests running
+ // in parallel. Since POI 6.0.0 this threshold also bounds how much of an entry with an
+ // unknown size (e.g. the data-descriptor entries POI itself writes to a stream, which
+ // the handlers re-read) is buffered on the heap before it is spilled to a temp file.
+ ZipInputStreamZipEntrySource.setThresholdBytesForTempFiles(2_000_000);
}
public static final String[] SCAN_EXCLUDES = {
diff --git a/poi-integration/src/test/java/org/apache/poi/stress/XSSFBFileHandler.java b/poi-integration/src/test/java/org/apache/poi/stress/XSSFBFileHandler.java
index 862e89be33..190ec6ea3a 100644
--- a/poi-integration/src/test/java/org/apache/poi/stress/XSSFBFileHandler.java
+++ b/poi-integration/src/test/java/org/apache/poi/stress/XSSFBFileHandler.java
@@ -45,7 +45,9 @@ public class XSSFBFileHandler extends AbstractFileHandler {
testOne(opcPackage);
}
- testNotHandledByWorkbookException(OPCPackage.open(UnsynchronizedByteArrayInputStream.builder().setByteArray(bytes).get()));
+ try (OPCPackage opcPackage = OPCPackage.open(UnsynchronizedByteArrayInputStream.builder().setByteArray(bytes).get())) {
+ testNotHandledByWorkbookException(opcPackage);
+ }
}
private void testNotHandledByWorkbookException(OPCPackage pkg) throws IOException {
diff --git a/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java b/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java
index adebc94508..72a0c9c55a 100644
--- a/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java
+++ b/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java
@@ -95,7 +95,11 @@ public class XSSFFileHandler extends SpreadsheetHandler {
is.close();
poifs.close();
}
- checkXSSFReader(OPCPackage.open(out.toInputStream()));
+ // close the package again before opening the workbook: a package opened from a
+ // stream keeps every (decompressed) part on the heap until it is closed
+ try (OPCPackage pkg = OPCPackage.open(out.toInputStream())) {
+ checkXSSFReader(pkg);
+ }
wb = new XSSFWorkbook(out.toInputStream());
}
diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java b/poi/src/main/java/org/apache/poi/util/IOUtils.java
index 570379fe59..6fe3dc1df5 100644
--- a/poi/src/main/java/org/apache/poi/util/IOUtils.java
+++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java
@@ -348,6 +348,28 @@ public final class IOUtils {
final int derivedLen = isLengthKnown && length >= 0 ? Math.min(length, derivedMaxLength) : derivedMaxLength;
final int byteArrayInitLen = calculateByteArrayInitLength(isLengthKnown, length, derivedMaxLength, maxInitBufferSize);
+ if (isLengthKnown && length >= 0 && byteArrayInitLen == derivedLen) {
+ // The whole payload fits the initial allocation, so read it straight into the result
+ // array. Going through a ByteArrayOutputStream would hold a second full copy of the
+ // data while toByteArray() copies it out, doubling the transient memory of every
+ // sized read (zip entries, records, ...).
+ // Keep reading in DEFAULT_BUFFER_SIZE chunks as the buffered path does, so that
+ // streams which enforce limits per read (e.g. the zip-bomb ratio check) report
+ // the same byte counts as before.
+ final byte[] result = new byte[derivedLen];
+ int totalBytes = 0, readBytes;
+ do {
+ readBytes = stream.read(result, totalBytes, Math.min(DEFAULT_BUFFER_SIZE, derivedLen - totalBytes));
+ totalBytes += Math.max(readBytes, 0);
+ } while (totalBytes < derivedLen && readBytes > -1);
+ if (totalBytes == derivedLen) {
+ return result;
+ }
+ if (checkEOFException) {
+ throw new EOFException("unexpected EOF - expected len: " + derivedLen + " - actual len: " + totalBytes);
+ }
+ return Arrays.copyOf(result, totalBytes);
+ }
final int internalBufferLen = DEFAULT_BUFFER_SIZE;
try (UnsynchronizedByteArrayOutputStream baos = UnsynchronizedByteArrayOutputStream.builder().setBufferSize(byteArrayInitLen).get()) {
byte[] buffer = new byte[internalBufferLen];
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org