Skip to content

Commit 6895f57

Browse files
committed
PAYG: synchronize spilled/spillStream reads in flush/close (Aikido)
flush() and close() on TeeingServletOutputStream read `spilled` and `spillStream` while the writers (recordSingleByte, recordRange, spillToDisk) mutate them under `synchronized(PaygResponseBodyWrapper.this)`. Without matching synchronization on the read side, the JMM permits a stale-false `spilled` or a partially-published `spillStream`, which would manifest as a lost final flush or an NPE. In practice Tomcat's HTTP/1.1 connector dispatches a single request on one thread, but the wrapper is documented to outlive the dispatch thread (it survives into AsyncListener.onComplete on a container thread), so the cross-thread happens-before is real. Fix matches the existing writer pattern by reading under the outer PaygResponseBodyWrapper.this monitor. Caught by Aikido AI on #6519.
1 parent 8e66678 commit 6895f57

1 file changed

Lines changed: 14 additions & 4 deletions

File tree

app/saas/src/main/java/stirling/software/saas/payg/filter/PaygResponseBodyWrapper.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,26 @@ public void write(byte[] b, int off, int len) throws IOException {
213213
@Override
214214
public void flush() throws IOException {
215215
delegate.flush();
216-
if (spilled && spillStream != null) {
217-
spillStream.flush();
216+
// Read spilled / spillStream under the outer monitor so we observe the publication
217+
// written by spillToDisk() on another thread. The writers (recordSingleByte,
218+
// recordRange, spillToDisk) already mutate these fields under
219+
// synchronized(PaygResponseBodyWrapper.this); without matching locking here the
220+
// JMM permits stale reads (false `spilled`, null `spillStream`) and Aikido AI
221+
// flagged that gap.
222+
synchronized (PaygResponseBodyWrapper.this) {
223+
if (spilled && spillStream != null) {
224+
spillStream.flush();
225+
}
218226
}
219227
}
220228

221229
@Override
222230
public void close() throws IOException {
223231
delegate.close();
224-
if (spilled && spillStream != null) {
225-
spillStream.flush();
232+
synchronized (PaygResponseBodyWrapper.this) {
233+
if (spilled && spillStream != null) {
234+
spillStream.flush();
235+
}
226236
}
227237
}
228238

0 commit comments

Comments
 (0)