Skip to content

Commit 30c75ff

Browse files
committed
Avoid locking if not handling asynchronously
Avoid the overhead of locking in the ServletOutputStream wrapper if it is not an asynchronous request. See gh-32342
1 parent dd5fe68 commit 30c75ff

File tree

2 files changed

+47
-15
lines changed

2 files changed

+47
-15
lines changed

spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java

+44-11
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public StandardServletAsyncWebRequest(HttpServletRequest request, HttpServletRes
8787

8888
super(request, new LifecycleHttpServletResponse(response));
8989

90-
this.state = (previousRequest != null ? previousRequest.state : State.ACTIVE);
90+
this.state = (previousRequest != null ? previousRequest.state : State.NEW);
9191

9292
//noinspection DataFlowIssue
9393
((LifecycleHttpServletResponse) getResponse()).setAsyncWebRequest(this);
@@ -142,11 +142,17 @@ public void startAsync() {
142142
"or by adding \"<async-supported>true</async-supported>\" to servlet and " +
143143
"filter declarations in web.xml.");
144144

145-
Assert.state(!isAsyncComplete(), "Async processing has already completed");
146-
147145
if (isAsyncStarted()) {
148146
return;
149147
}
148+
149+
if (this.state == State.NEW) {
150+
this.state = State.ASYNC;
151+
}
152+
else {
153+
Assert.state(this.state == State.ASYNC, "Cannot start async: [" + this.state + "]");
154+
}
155+
150156
this.asyncContext = getRequest().startAsync(getRequest(), getResponse());
151157
this.asyncContext.addListener(this);
152158
if (this.timeout != null) {
@@ -190,7 +196,7 @@ public void onError(AsyncEvent event) throws IOException {
190196
}
191197

192198
private void transitionToErrorState() {
193-
if (this.state == State.ACTIVE) {
199+
if (!isAsyncComplete()) {
194200
this.state = State.ERROR;
195201
}
196202
}
@@ -323,15 +329,29 @@ public void close() throws IOException {
323329
}
324330

325331
private void obtainLockAndCheckState() throws AsyncRequestNotUsableException {
326-
if (!this.asyncWebRequest.stateLock.tryLock() || this.asyncWebRequest.state != State.ACTIVE) {
327-
throw new AsyncRequestNotUsableException("Response not usable after " +
328-
(this.asyncWebRequest.state == State.COMPLETED ?
329-
"async request completion" : "onError notification") + ".");
332+
if (state() != State.NEW) {
333+
stateLock().lock();
334+
if (state() != State.ASYNC) {
335+
stateLock().unlock();
336+
throw new AsyncRequestNotUsableException("Response not usable after " +
337+
(state() == State.COMPLETED ?
338+
"async request completion" : "onError notification") + ".");
339+
}
330340
}
331341
}
332342

333343
private void releaseLock() {
334-
this.asyncWebRequest.stateLock.unlock();
344+
if (state() != State.NEW) {
345+
stateLock().unlock();
346+
}
347+
}
348+
349+
private State state() {
350+
return this.asyncWebRequest.state;
351+
}
352+
353+
private ReentrantLock stateLock() {
354+
return this.asyncWebRequest.stateLock;
335355
}
336356

337357
private void handleIOException(IOException ex, String msg) throws AsyncRequestNotUsableException {
@@ -345,7 +365,10 @@ private void handleIOException(IOException ex, String msg) throws AsyncRequestNo
345365
/**
346366
* Represents a state for {@link StandardServletAsyncWebRequest} to be in.
347367
* <p><pre>
348-
* ACTIVE ----+
368+
* NEW
369+
* |
370+
* v
371+
* ASYNC----> +
349372
* | |
350373
* v |
351374
* ERROR |
@@ -357,7 +380,17 @@ private void handleIOException(IOException ex, String msg) throws AsyncRequestNo
357380
*/
358381
private enum State {
359382

360-
ACTIVE, ERROR, COMPLETED
383+
/** New request (thas may not do async handling). */
384+
NEW,
385+
386+
/** Async handling has started. */
387+
ASYNC,
388+
389+
/** onError notification received, or ServletOutputStream failed. */
390+
ERROR,
391+
392+
/** onComplete notification received. */
393+
COMPLETED
361394

362395
}
363396

spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -96,9 +96,8 @@ public void startAsyncNotSupported() throws Exception {
9696
@Test
9797
public void startAsyncAfterCompleted() throws Exception {
9898
this.asyncRequest.onComplete(new AsyncEvent(new MockAsyncContext(this.request, this.response)));
99-
assertThatIllegalStateException().isThrownBy(
100-
this.asyncRequest::startAsync)
101-
.withMessage("Async processing has already completed");
99+
assertThatIllegalStateException().isThrownBy(this.asyncRequest::startAsync)
100+
.withMessage("Cannot start async: [COMPLETED]");
102101
}
103102

104103
@Test

0 commit comments

Comments
 (0)