Polish "Reset cached AppendableByteArray before it is reused"

See gh-51156
This commit is contained in:
Stéphane Nicoll
2026-08-11 17:59:20 +02:00
parent fc6acdbc21
commit dafadea2ab
4 changed files with 55 additions and 39 deletions
@@ -28,6 +28,7 @@ import java.nio.charset.CodingErrorAction;
import org.jspecify.annotations.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.function.ThrowingConsumer;
/**
* {@link Appendable} implementation that can be used to return a byte array. Designed to
@@ -35,6 +36,7 @@ import org.springframework.util.Assert;
* cached buffer scoped to the thread.
*
* @author Phillip Webb
* @author Stephane Nicoll
*/
class AppendableByteArray implements Appendable {
@@ -112,7 +114,6 @@ class AppendableByteArray implements Appendable {
}
byte[] result = new byte[size];
System.arraycopy(this.out.array(), this.out.arrayOffset() + position, result, 0, size);
reset();
return result;
}
@@ -121,7 +122,26 @@ class AppendableByteArray implements Appendable {
this.encoder.reset();
}
static AppendableByteArray get(Charset charset) {
static byte[] toByteArray(Charset charset, ThrowingConsumer<Appendable> appendable) throws IOException {
Assert.notNull(charset, "'charset' must not be null");
Assert.notNull(appendable, "'appendable' must not be null");
AppendableByteArray appendableByteArray = get(charset);
try {
appendable.acceptWithException(appendableByteArray);
return appendableByteArray.toByteArray();
}
catch (IOException | RuntimeException ex) {
throw ex;
}
catch (Exception ex) {
throw new IllegalStateException(ex);
}
finally {
appendableByteArray.reset();
}
}
private static AppendableByteArray get(Charset charset) {
Assert.notNull(charset, "'charset' must not be null");
SoftReference<AppendableByteArray> cached = cache.get();
AppendableByteArray result = (cached != null) ? cached.get() : null;
@@ -129,12 +149,6 @@ class AppendableByteArray implements Appendable {
result = new AppendableByteArray(charset);
cache.set(new SoftReference<>(result));
}
else {
// The cached instance is reused, so it must be clean before it is
// handed out again. A previous use may have been abandoned part-way,
// for example when writing the value threw, leaving content behind.
result.reset();
}
return result;
}
@@ -76,9 +76,7 @@ public interface WritableJson {
default byte[] toByteArray(Charset charset) {
Assert.notNull(charset, "'charset' must not be null");
try {
AppendableByteArray appendable = AppendableByteArray.get(charset);
to(appendable);
return appendable.toByteArray();
return AppendableByteArray.toByteArray(charset, this::to);
}
catch (IOException ex) {
throw new UncheckedIOException(ex);
@@ -33,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link AppendableByteArray}.
*
* @author Phillip Webb
* @author Stephane Nicoll
*/
class AppendableByteArrayTests {
@@ -61,23 +62,32 @@ class AppendableByteArrayTests {
@Test
void writeUsingCache() throws IOException {
assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string));
assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string));
assertByteArray(StandardCharsets.UTF_16, AppendableByteArray::get, (appendable) -> appendable.append(string));
assertByteArray(StandardCharsets.UTF_16, AppendableByteArray::get, (appendable) -> appendable.append(string));
assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string));
testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string));
testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string));
testWriteUsingCache(StandardCharsets.UTF_16, (appendable) -> appendable.append(string));
testWriteUsingCache(StandardCharsets.UTF_16, (appendable) -> appendable.append(string));
testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string));
}
private void testWriteUsingCache(Charset charset, ThrowingConsumer<Appendable> action) throws IOException {
byte[] baseLine = createFreshByteArray(charset, action);
assertThat(AppendableByteArray.toByteArray(charset, action)).isEqualTo(baseLine);
}
@Test
void getWhenPreviousUseWasAbandonedReturnsCleanInstance() throws IOException {
AppendableByteArray abandoned = AppendableByteArray.get(StandardCharsets.UTF_8);
abandoned.append("partial content");
// The instance is never converted to a byte array, as happens when writing
// the value throws part-way through.
AppendableByteArray reused = AppendableByteArray.get(StandardCharsets.UTF_8);
assertThat(reused).isSameAs(abandoned);
reused.append("clean");
assertThat(reused.toByteArray()).isEqualTo("clean".getBytes(StandardCharsets.UTF_8));
void toByteArrayWhenPreviousUseWasAbandonedReturnsCleanInstance() throws IOException {
try {
AppendableByteArray.toByteArray(StandardCharsets.UTF_8, (appendable) -> {
appendable.append("partial content");
throw new IllegalStateException("Interrupted");
});
}
catch (IllegalStateException ex) {
// Ignore
}
byte[] reused = AppendableByteArray.toByteArray(StandardCharsets.UTF_8,
(appendable) -> appendable.append("clean"));
assertThat(reused).isEqualTo("clean".getBytes(StandardCharsets.UTF_8));
}
private void assertByteArray(Charset charset, ThrowingConsumer<Appendable> action) throws Exception {
@@ -91,13 +101,18 @@ class AppendableByteArrayTests {
private void assertByteArray(Charset charset, Function<Charset, AppendableByteArray> factory,
ThrowingConsumer<Appendable> action) throws IOException {
byte[] baseline = createFreshByteArray(charset, action);
AppendableByteArray appendableByteArray = factory.apply(charset);
action.accept(appendableByteArray);
assertThat(appendableByteArray.toByteArray()).isEqualTo(baseline);
}
private byte[] createFreshByteArray(Charset charset, ThrowingConsumer<Appendable> action) throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
try (OutputStreamWriter writer = new OutputStreamWriter(out, charset)) {
action.accept(writer);
}
AppendableByteArray appendableByteArray = factory.apply(charset);
action.accept(appendableByteArray);
assertThat(appendableByteArray.toByteArray()).isEqualTo(out.toByteArray());
return out.toByteArray();
}
}
@@ -66,17 +66,6 @@ class WritableJsonTests {
assertThat(writable.toByteArray()).isEqualTo("{}".getBytes());
}
@Test
void toByteArrayWhenPreviousWriteFailedDoesNotIncludePartialContent() {
WritableJson failing = (out) -> {
out.append("{\"partial\":");
throw new IllegalStateException("bad");
};
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(failing::toByteArray);
WritableJson healthy = (out) -> out.append("{\"ok\":true}");
assertThat(healthy.toByteArray()).isEqualTo("{\"ok\":true}".getBytes(StandardCharsets.UTF_8));
}
@Test
void toResourceWritesJson() throws Exception {
File file = new File(this.temp, "out.json");