Skip to content

Commit

Permalink
fix: ensure requestEnd clears Vaadin thread locals (#20687) (#20692)
Browse files Browse the repository at this point in the history
Makes sure that Vaadin thread locals are cleared even if something
fails durung requestEnd execution. It also wraps Vaadin interceptors
execution in a try/catch block to ensure all of them are invoked
and that potential failures does not affect the continuation of
requestEnd method.
  • Loading branch information
mcollovati authored Dec 13, 2024
1 parent b1a71f7 commit 5b0a847
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 12 deletions.
26 changes: 15 additions & 11 deletions flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
Original file line number Diff line number Diff line change
Expand Up @@ -1475,19 +1475,23 @@ public void requestStart(VaadinRequest request, VaadinResponse response) {
*/
public void requestEnd(VaadinRequest request, VaadinResponse response,
VaadinSession session) {
if (session != null) {
assert VaadinSession.getCurrent() == session;
session.lock();
try {
cleanupSession(session);
final long duration = (System.nanoTime() - (Long) request
.getAttribute(REQUEST_START_TIME_ATTRIBUTE)) / 1000000;
session.setLastRequestDuration(duration);
} finally {
session.unlock();
try {
if (session != null) {
assert VaadinSession.getCurrent() == session;
session.lock();
try {
cleanupSession(session);
final long duration = (System.nanoTime() - (Long) request
.getAttribute(REQUEST_START_TIME_ATTRIBUTE))
/ 1000000;
session.setLastRequestDuration(duration);
} finally {
session.unlock();
}
}
} finally {
CurrentInstance.clearAll();
}
CurrentInstance.clearAll();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.http.HttpSessionBindingEvent;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -87,6 +86,44 @@ protected List<RequestHandler> createRequestHandlers()
}
}

@Test
public void requestEnd_serviceFailure_threadLocalsCleared() {
MockVaadinServletService service = new MockVaadinServletService() {
@Override
void cleanupSession(VaadinSession session) {
throw new RuntimeException("BOOM");
}
};
service.init();

VaadinRequest request = Mockito.mock(VaadinRequest.class);
VaadinResponse response = Mockito.mock(VaadinResponse.class);
service.requestStart(request, response);

Assert.assertSame(service, VaadinService.getCurrent());
Assert.assertSame(request, VaadinRequest.getCurrent());
Assert.assertSame(response, VaadinResponse.getCurrent());

VaadinSession session = Mockito.mock(VaadinSession.class);
VaadinSession.setCurrent(session);

try {
service.requestEnd(request, response, session);
Assert.fail("Should have thrown an exception");
} catch (Exception e) {
Assert.assertNull("VaadinService.current",
VaadinService.getCurrent());
Assert.assertNull("VaadinSession.current",
VaadinSession.getCurrent());
Assert.assertNull("VaadinRequest.current",
VaadinRequest.getCurrent());
Assert.assertNull("VaadinResponse.current",
VaadinResponse.getCurrent());
} finally {
CurrentInstance.clearAll();
}
}

@Test
public void should_reported_routing_server() {

Expand Down

0 comments on commit 5b0a847

Please sign in to comment.