From a59f40e5ccdc1ea6c1aa3ac2054aaaed72b38635 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 7 Sep 2026 12:24:52 +0100 Subject: [PATCH] Close the .xsb output when a save is abandoned part-way through Every saveXxx() in SchemaTypeSystemImpl runs the same sequence: construct an XsbReader, write the component once to fill the string pool, call writeRealHeader() - which opens a Filer output stream - write it again, then writeEnd() to flush and close. Nothing guards the middle. If any write throws, and they all raise SchemaTypeLoaderException on an IO error, writeEnd() is skipped and the .xsb output stream is left open with a partial file behind it. With scomp writing one file per global type, a failure part-way through a large schema leaks a descriptor for each of them. Add XsbReader.closeOutputQuietly(), which releases a stream a completed writeEnd() has already cleared, and call it from a finally in all eight save methods. Closing quietly keeps the original failure from being masked. Co-Authored-By: Claude Opus 5 (1M context) --- .../impl/schema/SchemaTypeSystemImpl.java | 104 ++++++++++++------ .../xmlbeans/impl/schema/XsbReader.java | 17 +++ .../impl/schema/XsbSaveStreamTest.java | 76 +++++++++++++ 3 files changed, 161 insertions(+), 36 deletions(-) create mode 100644 src/test/java/org/apache/xmlbeans/impl/schema/XsbSaveStreamTest.java diff --git a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaTypeSystemImpl.java b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaTypeSystemImpl.java index 510d1d940..fc7d445dd 100644 --- a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaTypeSystemImpl.java +++ b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaTypeSystemImpl.java @@ -305,10 +305,14 @@ private void initFromHeader() { void saveIndex() { String handle = "index"; XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeIndexData(); - saver.writeRealHeader(handle, FILETYPE_SCHEMAINDEX); - saver.writeIndexData(); - saver.writeEnd(); + try { + saver.writeIndexData(); + saver.writeRealHeader(handle, FILETYPE_SCHEMAINDEX); + saver.writeIndexData(); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } void savePointers() { @@ -345,10 +349,14 @@ void savePointersForNamespaces(Set namespaces, String dir) { void savePointerFile(String filename, String name) { XsbReader saver = new XsbReader(getTypeSystem(), filename); - saver.writeString(name); - saver.writeRealHeader(filename, FILETYPE_SCHEMAPOINTER); - saver.writeString(name); - saver.writeEnd(); + try { + saver.writeString(name); + saver.writeRealHeader(filename, FILETYPE_SCHEMAPOINTER); + saver.writeString(name); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } private Map buildTypeRefsByClassname(Map typesByClassname) { @@ -827,12 +835,16 @@ public void saveGlobalElement(SchemaGlobalElement elt) { } String handle = _localHandles.handleForElement(elt); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeParticleData((SchemaParticle) elt); - saver.writeString(elt.getSourceName()); - saver.writeRealHeader(handle, FILETYPE_SCHEMAELEMENT); - saver.writeParticleData((SchemaParticle) elt); - saver.writeString(elt.getSourceName()); - saver.writeEnd(); + try { + saver.writeParticleData((SchemaParticle) elt); + saver.writeString(elt.getSourceName()); + saver.writeRealHeader(handle, FILETYPE_SCHEMAELEMENT); + saver.writeParticleData((SchemaParticle) elt); + saver.writeString(elt.getSourceName()); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } public void saveGlobalAttribute(SchemaGlobalAttribute attr) { @@ -841,12 +853,16 @@ public void saveGlobalAttribute(SchemaGlobalAttribute attr) { } String handle = _localHandles.handleForAttribute(attr); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeAttributeData(attr); - saver.writeString(attr.getSourceName()); - saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTE); - saver.writeAttributeData(attr); - saver.writeString(attr.getSourceName()); - saver.writeEnd(); + try { + saver.writeAttributeData(attr); + saver.writeString(attr.getSourceName()); + saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTE); + saver.writeAttributeData(attr); + saver.writeString(attr.getSourceName()); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } public void saveModelGroup(SchemaModelGroup grp) { @@ -855,10 +871,14 @@ public void saveModelGroup(SchemaModelGroup grp) { } String handle = _localHandles.handleForModelGroup(grp); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeModelGroupData(grp); - saver.writeRealHeader(handle, FILETYPE_SCHEMAMODELGROUP); - saver.writeModelGroupData(grp); - saver.writeEnd(); + try { + saver.writeModelGroupData(grp); + saver.writeRealHeader(handle, FILETYPE_SCHEMAMODELGROUP); + saver.writeModelGroupData(grp); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } public void saveAttributeGroup(SchemaAttributeGroup grp) { @@ -867,10 +887,14 @@ public void saveAttributeGroup(SchemaAttributeGroup grp) { } String handle = _localHandles.handleForAttributeGroup(grp); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeAttributeGroupData(grp); - saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTEGROUP); - saver.writeAttributeGroupData(grp); - saver.writeEnd(); + try { + saver.writeAttributeGroupData(grp); + saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTEGROUP); + saver.writeAttributeGroupData(grp); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } public void saveIdentityConstraint(SchemaIdentityConstraint idc) { @@ -879,19 +903,27 @@ public void saveIdentityConstraint(SchemaIdentityConstraint idc) { } String handle = _localHandles.handleForIdentityConstraint(idc); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeIdConstraintData(idc); - saver.writeRealHeader(handle, FILETYPE_SCHEMAIDENTITYCONSTRAINT); - saver.writeIdConstraintData(idc); - saver.writeEnd(); + try { + saver.writeIdConstraintData(idc); + saver.writeRealHeader(handle, FILETYPE_SCHEMAIDENTITYCONSTRAINT); + saver.writeIdConstraintData(idc); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } void saveType(SchemaType type) { String handle = _localHandles.handleForType(type); XsbReader saver = new XsbReader(getTypeSystem(), handle); - saver.writeTypeData(type); - saver.writeRealHeader(handle, FILETYPE_SCHEMATYPE); - saver.writeTypeData(type); - saver.writeEnd(); + try { + saver.writeTypeData(type); + saver.writeRealHeader(handle, FILETYPE_SCHEMATYPE); + saver.writeTypeData(type); + saver.writeEnd(); + } finally { + saver.closeOutputQuietly(); + } } public static String crackPointer(InputStream stream) { diff --git a/src/main/java/org/apache/xmlbeans/impl/schema/XsbReader.java b/src/main/java/org/apache/xmlbeans/impl/schema/XsbReader.java index 9094a2164..88d232c87 100644 --- a/src/main/java/org/apache/xmlbeans/impl/schema/XsbReader.java +++ b/src/main/java/org/apache/xmlbeans/impl/schema/XsbReader.java @@ -181,6 +181,23 @@ void readEnd() { _handle = null; } + /** + * Releases an output stream left open by a write that was abandoned part-way + * through. A completed writeEnd() has already cleared it, so this does nothing. + */ + void closeOutputQuietly() { + if (_output != null) { + try { + _output.close(); + } catch (IOException e) { + // the caller is already unwinding - don't mask the real failure + } + _output = null; + _stringPool = null; + _handle = null; + } + } + void writeEnd() { try { if (_output != null) { diff --git a/src/test/java/org/apache/xmlbeans/impl/schema/XsbSaveStreamTest.java b/src/test/java/org/apache/xmlbeans/impl/schema/XsbSaveStreamTest.java new file mode 100644 index 000000000..54b881264 --- /dev/null +++ b/src/test/java/org/apache/xmlbeans/impl/schema/XsbSaveStreamTest.java @@ -0,0 +1,76 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.xmlbeans.impl.schema; + +import org.apache.xmlbeans.Filer; +import org.apache.xmlbeans.SchemaTypeLoaderException; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.Writer; +import java.lang.reflect.Field; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class XsbSaveStreamTest { + + // Stands in for a full disk or a revoked permission: the file opens, then every + // write fails. + private static class FailingOutputStream extends OutputStream { + private boolean closed; + + @Override + public void write(int b) throws IOException { + throw new IOException("disk full"); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + throw new IOException("disk full"); + } + + @Override + public void close() { + closed = true; + } + } + + @Test + void closesBinaryFileWhenTheWriteFails() throws Exception { + FailingOutputStream stream = new FailingOutputStream(); + + SchemaTypeSystemImpl typeSystem = new SchemaTypeSystemImpl("test"); + Field filerF = SchemaTypeSystemImpl.class.getDeclaredField("_filer"); + filerF.setAccessible(true); + filerF.set(typeSystem, new Filer() { + @Override + public OutputStream createBinaryFile(String typename) { + return stream; + } + + @Override + public Writer createSourceFile(String typename, String sourceCodeEncoding) { + throw new UnsupportedOperationException(); + } + }); + + assertThrows(SchemaTypeLoaderException.class, () -> typeSystem.savePointerFile("p", "test")); + assertTrue(stream.closed, "the abandoned .xsb output should have been closed"); + } +}