Skip to content

Commit c863d21

Browse files
pjfanningclaude
andauthored
Close the .xsb output when a save is abandoned part-way through (#112)
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) <noreply@anthropic.com>
1 parent c0c0c65 commit c863d21

3 files changed

Lines changed: 161 additions & 36 deletions

File tree

src/main/java/org/apache/xmlbeans/impl/schema/SchemaTypeSystemImpl.java

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,14 @@ private void initFromHeader() {
305305
void saveIndex() {
306306
String handle = "index";
307307
XsbReader saver = new XsbReader(getTypeSystem(), handle);
308-
saver.writeIndexData();
309-
saver.writeRealHeader(handle, FILETYPE_SCHEMAINDEX);
310-
saver.writeIndexData();
311-
saver.writeEnd();
308+
try {
309+
saver.writeIndexData();
310+
saver.writeRealHeader(handle, FILETYPE_SCHEMAINDEX);
311+
saver.writeIndexData();
312+
saver.writeEnd();
313+
} finally {
314+
saver.closeOutputQuietly();
315+
}
312316
}
313317

314318
void savePointers() {
@@ -345,10 +349,14 @@ void savePointersForNamespaces(Set<String> namespaces, String dir) {
345349

346350
void savePointerFile(String filename, String name) {
347351
XsbReader saver = new XsbReader(getTypeSystem(), filename);
348-
saver.writeString(name);
349-
saver.writeRealHeader(filename, FILETYPE_SCHEMAPOINTER);
350-
saver.writeString(name);
351-
saver.writeEnd();
352+
try {
353+
saver.writeString(name);
354+
saver.writeRealHeader(filename, FILETYPE_SCHEMAPOINTER);
355+
saver.writeString(name);
356+
saver.writeEnd();
357+
} finally {
358+
saver.closeOutputQuietly();
359+
}
352360
}
353361

354362
private Map<String, SchemaComponent.Ref> buildTypeRefsByClassname(Map<String, SchemaType> typesByClassname) {
@@ -827,12 +835,16 @@ public void saveGlobalElement(SchemaGlobalElement elt) {
827835
}
828836
String handle = _localHandles.handleForElement(elt);
829837
XsbReader saver = new XsbReader(getTypeSystem(), handle);
830-
saver.writeParticleData((SchemaParticle) elt);
831-
saver.writeString(elt.getSourceName());
832-
saver.writeRealHeader(handle, FILETYPE_SCHEMAELEMENT);
833-
saver.writeParticleData((SchemaParticle) elt);
834-
saver.writeString(elt.getSourceName());
835-
saver.writeEnd();
838+
try {
839+
saver.writeParticleData((SchemaParticle) elt);
840+
saver.writeString(elt.getSourceName());
841+
saver.writeRealHeader(handle, FILETYPE_SCHEMAELEMENT);
842+
saver.writeParticleData((SchemaParticle) elt);
843+
saver.writeString(elt.getSourceName());
844+
saver.writeEnd();
845+
} finally {
846+
saver.closeOutputQuietly();
847+
}
836848
}
837849

838850
public void saveGlobalAttribute(SchemaGlobalAttribute attr) {
@@ -841,12 +853,16 @@ public void saveGlobalAttribute(SchemaGlobalAttribute attr) {
841853
}
842854
String handle = _localHandles.handleForAttribute(attr);
843855
XsbReader saver = new XsbReader(getTypeSystem(), handle);
844-
saver.writeAttributeData(attr);
845-
saver.writeString(attr.getSourceName());
846-
saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTE);
847-
saver.writeAttributeData(attr);
848-
saver.writeString(attr.getSourceName());
849-
saver.writeEnd();
856+
try {
857+
saver.writeAttributeData(attr);
858+
saver.writeString(attr.getSourceName());
859+
saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTE);
860+
saver.writeAttributeData(attr);
861+
saver.writeString(attr.getSourceName());
862+
saver.writeEnd();
863+
} finally {
864+
saver.closeOutputQuietly();
865+
}
850866
}
851867

852868
public void saveModelGroup(SchemaModelGroup grp) {
@@ -855,10 +871,14 @@ public void saveModelGroup(SchemaModelGroup grp) {
855871
}
856872
String handle = _localHandles.handleForModelGroup(grp);
857873
XsbReader saver = new XsbReader(getTypeSystem(), handle);
858-
saver.writeModelGroupData(grp);
859-
saver.writeRealHeader(handle, FILETYPE_SCHEMAMODELGROUP);
860-
saver.writeModelGroupData(grp);
861-
saver.writeEnd();
874+
try {
875+
saver.writeModelGroupData(grp);
876+
saver.writeRealHeader(handle, FILETYPE_SCHEMAMODELGROUP);
877+
saver.writeModelGroupData(grp);
878+
saver.writeEnd();
879+
} finally {
880+
saver.closeOutputQuietly();
881+
}
862882
}
863883

864884
public void saveAttributeGroup(SchemaAttributeGroup grp) {
@@ -867,10 +887,14 @@ public void saveAttributeGroup(SchemaAttributeGroup grp) {
867887
}
868888
String handle = _localHandles.handleForAttributeGroup(grp);
869889
XsbReader saver = new XsbReader(getTypeSystem(), handle);
870-
saver.writeAttributeGroupData(grp);
871-
saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTEGROUP);
872-
saver.writeAttributeGroupData(grp);
873-
saver.writeEnd();
890+
try {
891+
saver.writeAttributeGroupData(grp);
892+
saver.writeRealHeader(handle, FILETYPE_SCHEMAATTRIBUTEGROUP);
893+
saver.writeAttributeGroupData(grp);
894+
saver.writeEnd();
895+
} finally {
896+
saver.closeOutputQuietly();
897+
}
874898
}
875899

876900
public void saveIdentityConstraint(SchemaIdentityConstraint idc) {
@@ -879,19 +903,27 @@ public void saveIdentityConstraint(SchemaIdentityConstraint idc) {
879903
}
880904
String handle = _localHandles.handleForIdentityConstraint(idc);
881905
XsbReader saver = new XsbReader(getTypeSystem(), handle);
882-
saver.writeIdConstraintData(idc);
883-
saver.writeRealHeader(handle, FILETYPE_SCHEMAIDENTITYCONSTRAINT);
884-
saver.writeIdConstraintData(idc);
885-
saver.writeEnd();
906+
try {
907+
saver.writeIdConstraintData(idc);
908+
saver.writeRealHeader(handle, FILETYPE_SCHEMAIDENTITYCONSTRAINT);
909+
saver.writeIdConstraintData(idc);
910+
saver.writeEnd();
911+
} finally {
912+
saver.closeOutputQuietly();
913+
}
886914
}
887915

888916
void saveType(SchemaType type) {
889917
String handle = _localHandles.handleForType(type);
890918
XsbReader saver = new XsbReader(getTypeSystem(), handle);
891-
saver.writeTypeData(type);
892-
saver.writeRealHeader(handle, FILETYPE_SCHEMATYPE);
893-
saver.writeTypeData(type);
894-
saver.writeEnd();
919+
try {
920+
saver.writeTypeData(type);
921+
saver.writeRealHeader(handle, FILETYPE_SCHEMATYPE);
922+
saver.writeTypeData(type);
923+
saver.writeEnd();
924+
} finally {
925+
saver.closeOutputQuietly();
926+
}
895927
}
896928

897929
public static String crackPointer(InputStream stream) {

src/main/java/org/apache/xmlbeans/impl/schema/XsbReader.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,23 @@ void readEnd() {
195195
_handle = null;
196196
}
197197

198+
/**
199+
* Releases an output stream left open by a write that was abandoned part-way
200+
* through. A completed writeEnd() has already cleared it, so this does nothing.
201+
*/
202+
void closeOutputQuietly() {
203+
if (_output != null) {
204+
try {
205+
_output.close();
206+
} catch (IOException e) {
207+
// the caller is already unwinding - don't mask the real failure
208+
}
209+
_output = null;
210+
_stringPool = null;
211+
_handle = null;
212+
}
213+
}
214+
198215
void writeEnd() {
199216
try {
200217
if (_output != null) {
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/* Licensed to the Apache Software Foundation (ASF) under one or more
2+
* contributor license agreements. See the NOTICE file distributed with
3+
* this work for additional information regarding copyright ownership.
4+
* The ASF licenses this file to You under the Apache License, Version 2.0
5+
* (the "License"); you may not use this file except in compliance with
6+
* the License. You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.apache.xmlbeans.impl.schema;
18+
19+
import org.apache.xmlbeans.Filer;
20+
import org.apache.xmlbeans.SchemaTypeLoaderException;
21+
import org.junit.jupiter.api.Test;
22+
23+
import java.io.IOException;
24+
import java.io.OutputStream;
25+
import java.io.Writer;
26+
import java.lang.reflect.Field;
27+
28+
import static org.junit.jupiter.api.Assertions.assertThrows;
29+
import static org.junit.jupiter.api.Assertions.assertTrue;
30+
31+
public class XsbSaveStreamTest {
32+
33+
// Stands in for a full disk or a revoked permission: the file opens, then every
34+
// write fails.
35+
private static class FailingOutputStream extends OutputStream {
36+
private boolean closed;
37+
38+
@Override
39+
public void write(int b) throws IOException {
40+
throw new IOException("disk full");
41+
}
42+
43+
@Override
44+
public void write(byte[] b, int off, int len) throws IOException {
45+
throw new IOException("disk full");
46+
}
47+
48+
@Override
49+
public void close() {
50+
closed = true;
51+
}
52+
}
53+
54+
@Test
55+
void closesBinaryFileWhenTheWriteFails() throws Exception {
56+
FailingOutputStream stream = new FailingOutputStream();
57+
58+
SchemaTypeSystemImpl typeSystem = new SchemaTypeSystemImpl("test");
59+
Field filerF = SchemaTypeSystemImpl.class.getDeclaredField("_filer");
60+
filerF.setAccessible(true);
61+
filerF.set(typeSystem, new Filer() {
62+
@Override
63+
public OutputStream createBinaryFile(String typename) {
64+
return stream;
65+
}
66+
67+
@Override
68+
public Writer createSourceFile(String typename, String sourceCodeEncoding) {
69+
throw new UnsupportedOperationException();
70+
}
71+
});
72+
73+
assertThrows(SchemaTypeLoaderException.class, () -> typeSystem.savePointerFile("p", "test"));
74+
assertTrue(stream.closed, "the abandoned .xsb output should have been closed");
75+
}
76+
}

0 commit comments

Comments
 (0)