-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Description
Bug description
When StaxEventItemWriter is used within the same transaction (TransactionTemplate) in the following pattern, problems occur:
- Using the same
StaxEventItemWriterinstance setResource(r1) -> open -> write -> closesetResource(r2) -> open -> write -> closesetResource(r3) -> open -> write -> close
Observed problems (depending on the environment, one or both may occur):
java.nio.channels.ClosedChannelExceptionat transaction commit (or at the end of transaction synchronization)- Some
FileChannels opened for r1/r2/r3 remain open after the transaction ends (resource leak)
Related issue:
Environment
- Spring Batch version: 6.0.x, 5.2.x
Steps to reproduce
- Add the following two tests to Spring Batch codebase in
org.springframework.batch.infrastructure.item.xml.TransactionalStaxEventItemWriterTests. - Run tests. You will observe either:
ClosedChannelExceptioninshouldWriteThreeSeparateFilesWhenMultipleOpenCloseAndResourceSwitchInSingleTransaction, or- a failure in
shouldCloseAllFileChannelsAfterTransactionbecause some channels remainisOpen() == trueafter transaction completion.
Expected behavior
Even when switching resources and opening/closing the writer multiple times within the same transaction:
- No
ClosedChannelExceptionshould be thrown at transaction completion. - After the transaction ends (commit/rollback), all FileChannels opened during that transaction must be closed.
Minimal Complete Reproducible example
The following tests validate two aspects:
- Exception reproduction:
shouldWriteThreeSeparateFilesWhenMultipleOpenCloseAndResourceSwitchInSingleTransaction - Leak reproduction:
shouldCloseAllFileChannelsAfterTransaction- It uses reflection to extract the underlying
FileChanneland checksisOpen()after the transaction.
- It uses reflection to extract the underlying
@Test
void shouldWriteThreeSeparateFilesWhenMultipleOpenCloseAndResourceSwitchInSingleTransaction() throws Exception {
WritableResource r1 = new FileSystemResource(File.createTempFile("stax-tx-rot-1", ".xml"));
WritableResource r2 = new FileSystemResource(File.createTempFile("stax-tx-rot-2", ".xml"));
WritableResource r3 = new FileSystemResource(File.createTempFile("stax-tx-rot-3", ".xml"));
assertDoesNotThrow(() ->
new TransactionTemplate(transactionManager).execute((TransactionCallback<Void>) status -> {
try {
writer.setResource(r1);
writer.open(new ExecutionContext());
writer.write(items);
writer.close();
writer.setResource(r2);
writer.open(new ExecutionContext());
writer.write(items);
writer.close();
writer.setResource(r3);
writer.open(new ExecutionContext());
writer.write(items);
writer.close();
return null;
}
catch (Exception e) {
throw new RuntimeException(e);
}
})
);
}
@Test
void shouldCloseAllFileChannelsAfterTransaction() throws Exception {
WritableResource r1 = new FileSystemResource(File.createTempFile("stax-tx-leak-1", ".xml"));
WritableResource r2 = new FileSystemResource(File.createTempFile("stax-tx-leak-2", ".xml"));
WritableResource r3 = new FileSystemResource(File.createTempFile("stax-tx-leak-3", ".xml"));
List<FileChannel> opened = new ArrayList<>();
try {
new TransactionTemplate(transactionManager).execute((TransactionCallback<Void>) status -> {
try {
writer.setResource(r1);
writer.open(new ExecutionContext());
FileChannel ch1 = extractChannelFromStaxWriter(writer);
assertNotNull(ch1);
opened.add(ch1);
writer.write(items);
writer.close();
writer.setResource(r2);
writer.open(new ExecutionContext());
FileChannel ch2 = extractChannelFromStaxWriter(writer);
assertNotNull(ch2);
opened.add(ch2);
writer.write(items);
writer.close();
writer.setResource(r3);
writer.open(new ExecutionContext());
FileChannel ch3 = extractChannelFromStaxWriter(writer);
assertNotNull(ch3);
opened.add(ch3);
writer.write(items);
writer.close();
return null;
}
catch (Exception ignored) {
}
});
}
catch (Exception ignored) {
// Continue to check leaks even if an exception happens
}
assertEquals(3, opened.size(), "Expected 3 opened channels");
for (FileChannel ch : opened) {
assertFalse(ch.isOpen(), "FileChannel should be closed after transaction");
}
}
private static FileChannel extractChannelFromStaxWriter(StaxEventItemWriter<?> w) throws Exception {
// legacy version
Field field = StaxEventItemWriter.class.getDeclaredField("channel");
field.setAccessible(true);
return (FileChannel) field.get(w);
// enhance version
Spring Batch 6.x layout: StaxEventItemWriter.state.channel
Field stateField = StaxEventItemWriter.class.getDeclaredField("state");
stateField.setAccessible(true);
Object state = stateField.get(w);
Field channelField = state.getClass().getDeclaredField("channel");
channelField.setAccessible(true);
return (FileChannel) channelField.get(state);
}Observed stacktrace example
org.springframework.batch.infrastructure.support.transaction.FlushFailedException: Could not write to output buffer
Caused by: java.nio.channels.ClosedChannelException
Why this happens
The key is that TransactionAwareBufferedWriter performs flush/close at transaction synchronization time.
Problematic structure:
- It registers a close callback like
TransactionAwareBufferedWriter(fileChannel, this::closeStream) - But
closeStream()closes the writer instance’s mutable field (e.g.channel) rather than closing the specificfileChannelthat was used when the callback was registered - Within the same transaction, repeated
open()calls overwrite thechannelfield as resources are switched - At transaction completion, the callback may:
- close only the last channel (leaving earlier channels open), and/or
- attempt to flush/write using a channel that is already closed, causing
ClosedChannelException
Suggested fix direction
To make this safe, the resources created by a single open() (e.g. FileOutputStream/FileChannel/Writer/XMLEventWriter) should be encapsulated in a state object, and the TransactionAwareBufferedWriter close callback should be bound to that specific state instance.
In short:
- Introduce an
OutputStateinStaxEventItemWriterto own those resources - Register the transactional close callback as
TransactionAwareBufferedWriter(fileChannel, state::closeStream) - In
close(), callstate.close(...)and then setstate = null
Reference / similar design in codebase
AbstractFileItemWriter uses an OutputState to encapsulate stream/channel lifecycle and binds the transactional writer close callback to that state, avoiding the same class of problems.