Skip to content

Commit 195150b

Browse files
LinzyooLinzyoo
authored
fix issue 2485 which occur oom when using async servlet request. (#3440)
* fix issue 2485 which occur oom when using async servlet request. * optimize imports * 1. fix the same issue in the webmvc-v6x 2. improve based on review comments
1 parent b78b09d commit 195150b

File tree

7 files changed

+142
-17
lines changed

7 files changed

+142
-17
lines changed

‎sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
import com.alibaba.csp.sentinel.slots.block.BlockException;
2727
import com.alibaba.csp.sentinel.util.AssertUtil;
2828
import com.alibaba.csp.sentinel.util.StringUtil;
29+
2930
import org.springframework.web.context.request.RequestContextHolder;
3031
import org.springframework.web.context.request.ServletRequestAttributes;
31-
import org.springframework.web.servlet.HandlerInterceptor;
32+
import org.springframework.web.method.HandlerMethod;
33+
import org.springframework.web.servlet.AsyncHandlerInterceptor;
3234
import org.springframework.web.servlet.ModelAndView;
3335

3436
import javax.servlet.http.HttpServletRequest;
@@ -50,11 +52,11 @@
5052
* return mav;
5153
* }
5254
* </pre>
53-
*
55+
*
5456
* @author kaizi2009
5557
* @since 1.7.1
5658
*/
57-
public abstract class AbstractSentinelInterceptor implements HandlerInterceptor {
59+
public abstract class AbstractSentinelInterceptor implements AsyncHandlerInterceptor {
5860

5961
public static final String SENTINEL_SPRING_WEB_CONTEXT_NAME = "sentinel_spring_web_context";
6062
private static final String EMPTY_ORIGIN = "";
@@ -66,12 +68,12 @@ public AbstractSentinelInterceptor(BaseWebMvcConfig config) {
6668
AssertUtil.assertNotBlank(config.getRequestAttributeName(), "requestAttributeName should not be blank");
6769
this.baseWebMvcConfig = config;
6870
}
69-
71+
7072
/**
7173
* @param request
7274
* @param rcKey
7375
* @param step
74-
* @return reference count after increasing (initial value as zero to be increased)
76+
* @return reference count after increasing (initial value as zero to be increased)
7577
*/
7678
private Integer increaseReference(HttpServletRequest request, String rcKey, int step) {
7779
Object obj = request.getAttribute(rcKey);
@@ -85,10 +87,10 @@ private Integer increaseReference(HttpServletRequest request, String rcKey, int
8587
request.setAttribute(rcKey, newRc);
8688
return newRc;
8789
}
88-
90+
8991
@Override
9092
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
91-
throws Exception {
93+
throws Exception {
9294
try {
9395
String resourceName = getResourceName(request);
9496

@@ -99,7 +101,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
99101
if (increaseReference(request, this.baseWebMvcConfig.getRequestRefName(), 1) != 1) {
100102
return true;
101103
}
102-
104+
103105
// Parse the request origin using registered origin parser.
104106
String origin = parseOrigin(request);
105107
String contextName = getContextName(request);
@@ -135,21 +137,45 @@ protected String getContextName(HttpServletRequest request) {
135137
return SENTINEL_SPRING_WEB_CONTEXT_NAME;
136138
}
137139

140+
141+
/**
142+
* When a handler starts an asynchronous request, the DispatcherServlet exits without invoking postHandle and afterCompletion
143+
* Called instead of postHandle and afterCompletion to exit the context and clean thread-local variables when the handler is being executed concurrently.
144+
*
145+
* @param request the current request
146+
* @param response the current response
147+
* @param handler the handler (or {@link HandlerMethod}) that started async
148+
* execution, for type and/or instance examination
149+
*/
150+
@Override
151+
public void afterConcurrentHandlingStarted(HttpServletRequest request, HttpServletResponse response,
152+
Object handler) throws Exception {
153+
exit(request);
154+
}
155+
138156
@Override
139157
public void afterCompletion(HttpServletRequest request, HttpServletResponse response,
140158
Object handler, Exception ex) throws Exception {
159+
exit(request, ex);
160+
}
161+
162+
private void exit(HttpServletRequest request) {
163+
exit(request, null);
164+
}
165+
166+
private void exit(HttpServletRequest request, Exception ex) {
141167
if (increaseReference(request, this.baseWebMvcConfig.getRequestRefName(), -1) != 0) {
142168
return;
143169
}
144-
170+
145171
Entry entry = getEntryInRequest(request, baseWebMvcConfig.getRequestAttributeName());
146172
if (entry == null) {
147173
// should not happen
148174
RecordLog.warn("[{}] No entry found in request, key: {}",
149175
getClass().getSimpleName(), baseWebMvcConfig.getRequestAttributeName());
150176
return;
151177
}
152-
178+
153179
traceExceptionAndExit(entry, ex);
154180
removeEntryInRequest(request);
155181
ContextUtil.exit();
@@ -162,7 +188,7 @@ public void postHandle(HttpServletRequest request, HttpServletResponse response,
162188

163189
protected Entry getEntryInRequest(HttpServletRequest request, String attrKey) {
164190
Object entryObject = request.getAttribute(attrKey);
165-
return entryObject == null ? null : (Entry)entryObject;
191+
return entryObject == null ? null : (Entry) entryObject;
166192
}
167193

168194
protected void removeEntryInRequest(HttpServletRequest request) {
@@ -188,7 +214,7 @@ && increaseReference(request, this.baseWebMvcConfig.getRequestRefName() + ":" +
188214
}
189215

190216
protected void handleBlockException(HttpServletRequest request, HttpServletResponse response, BlockException e)
191-
throws Exception {
217+
throws Exception {
192218
if (baseWebMvcConfig.getBlockExceptionHandler() != null) {
193219
baseWebMvcConfig.getBlockExceptionHandler().handle(request, response, e);
194220
} else {

‎sentinel-adapter/sentinel-spring-webmvc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/SentinelSpringMvcIntegrationTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import static org.junit.Assert.assertEquals;
1919
import static org.junit.Assert.assertNotNull;
20+
import static org.junit.Assert.assertNull;
2021
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
2122
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
2223
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
2324

25+
import com.alibaba.csp.sentinel.context.ContextUtil;
2426
import com.alibaba.csp.sentinel.node.ClusterNode;
2527
import com.alibaba.csp.sentinel.slots.block.RuleConstant;
2628
import com.alibaba.csp.sentinel.slots.block.degrade.DegradeRule;
@@ -66,6 +68,18 @@ public void testBase() throws Exception {
6668
assertEquals(1, cn.passQps(), 0.01);
6769
}
6870

71+
@Test
72+
public void testAsync() throws Exception {
73+
String url = "/async";
74+
this.mvc.perform(get(url))
75+
.andExpect(status().isOk());
76+
77+
ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
78+
assertNotNull(cn);
79+
assertEquals(1, cn.passQps(), 0.01);
80+
assertNull(ContextUtil.getContext());
81+
}
82+
6983
@Test
7084
public void testOriginParser() throws Exception {
7185
String springMvcPathVariableUrl = "/foo/{id}";

‎sentinel-adapter/sentinel-spring-webmvc-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/controller/TestController.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import com.alibaba.csp.sentinel.adapter.spring.webmvc.exception.BizException;
2020
import org.springframework.web.bind.annotation.GetMapping;
2121
import org.springframework.web.bind.annotation.PathVariable;
22+
import org.springframework.web.bind.annotation.ResponseBody;
2223
import org.springframework.web.bind.annotation.RestController;
24+
import org.springframework.web.context.request.async.DeferredResult;
2325

2426
/**
2527
* @author kaizi2009
@@ -58,4 +60,16 @@ public String apiExclude(@PathVariable("id") Long id) {
5860
return "Exclude " + id;
5961
}
6062

63+
@GetMapping("/async")
64+
@ResponseBody
65+
public DeferredResult<String> distribute() throws Exception{
66+
DeferredResult<String> result = new DeferredResult<>();
67+
68+
Thread thread = new Thread(() -> result.setResult("async result."));
69+
thread.start();
70+
71+
Thread.yield();
72+
return result;
73+
}
74+
6175
}

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/AbstractSentinelInterceptor.java

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
*/
1616
package com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x;
1717

18-
import com.alibaba.csp.sentinel.*;
18+
import com.alibaba.csp.sentinel.Entry;
19+
import com.alibaba.csp.sentinel.EntryType;
20+
import com.alibaba.csp.sentinel.ResourceTypeConstants;
21+
import com.alibaba.csp.sentinel.SphU;
22+
import com.alibaba.csp.sentinel.Tracer;
1923
import com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x.config.BaseWebMvcConfig;
2024
import com.alibaba.csp.sentinel.context.ContextUtil;
2125
import com.alibaba.csp.sentinel.log.RecordLog;
@@ -24,7 +28,8 @@
2428
import com.alibaba.csp.sentinel.util.StringUtil;
2529
import jakarta.servlet.http.HttpServletRequest;
2630
import jakarta.servlet.http.HttpServletResponse;
27-
import org.springframework.web.servlet.HandlerInterceptor;
31+
import org.springframework.web.method.HandlerMethod;
32+
import org.springframework.web.servlet.AsyncHandlerInterceptor;
2833
import org.springframework.web.servlet.ModelAndView;
2934

3035
/**
@@ -45,7 +50,7 @@
4550
*
4651
* @since 1.8.8
4752
*/
48-
public abstract class AbstractSentinelInterceptor implements HandlerInterceptor {
53+
public abstract class AbstractSentinelInterceptor implements AsyncHandlerInterceptor {
4954

5055
public static final String SENTINEL_SPRING_WEB_CONTEXT_NAME = "sentinel_spring_web_context";
5156
private static final String EMPTY_ORIGIN = "";
@@ -124,9 +129,33 @@ protected String getContextName(HttpServletRequest request) {
124129
return SENTINEL_SPRING_WEB_CONTEXT_NAME;
125130
}
126131

132+
133+
/**
134+
* When a handler starts an asynchronous request, the DispatcherServlet exits without invoking postHandle and afterCompletion
135+
* Called instead of postHandle and afterCompletion to exit the context and clean thread-local variables when the handler is being executed concurrently.
136+
*
137+
* @param request the current request
138+
* @param response the current response
139+
* @param handler the handler (or {@link HandlerMethod}) that started async
140+
* execution, for type and/or instance examination
141+
*/
142+
@Override
143+
public void afterConcurrentHandlingStarted(HttpServletRequest request, HttpServletResponse response,
144+
Object handler) throws Exception {
145+
exit(request);
146+
}
147+
127148
@Override
128149
public void afterCompletion(HttpServletRequest request, HttpServletResponse response,
129150
Object handler, Exception ex) throws Exception {
151+
exit(request, ex);
152+
}
153+
154+
private void exit(HttpServletRequest request) {
155+
exit(request, null);
156+
}
157+
158+
private void exit(HttpServletRequest request, Exception ex) {
130159
if (increaseReference(request, this.baseWebMvcConfig.getRequestRefName(), -1) != 0) {
131160
return;
132161
}

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/SentinelSpringMvcIntegrationTest.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import static org.junit.Assert.assertEquals;
1919
import static org.junit.Assert.assertNotNull;
20+
import static org.junit.Assert.assertNull;
2021
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
2122
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
2223
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
2324

25+
import com.alibaba.csp.sentinel.context.ContextUtil;
2426
import com.alibaba.csp.sentinel.node.ClusterNode;
2527
import com.alibaba.csp.sentinel.slots.block.RuleConstant;
2628
import com.alibaba.csp.sentinel.slots.block.flow.FlowRule;
@@ -64,6 +66,18 @@ public void testBase() throws Exception {
6466
assertEquals(1, cn.passQps(), 0.01);
6567
}
6668

69+
@Test
70+
public void testAsync() throws Exception {
71+
String url = "/async";
72+
this.mvc.perform(get(url))
73+
.andExpect(status().isOk());
74+
75+
ClusterNode cn = ClusterBuilderSlot.getClusterNode(url);
76+
assertNotNull(cn);
77+
assertEquals(1, cn.passQps(), 0.01);
78+
assertNull(ContextUtil.getContext());
79+
}
80+
6781
@Test
6882
public void testOriginParser() throws Exception {
6983
String springMvcPathVariableUrl = "/foo/{id}";
@@ -78,7 +92,7 @@ public void testOriginParser() throws Exception {
7892

7993
// This will be blocked since the caller is same: userA
8094
this.mvc.perform(
81-
get("/foo/2").accept(MediaType.APPLICATION_JSON).header(headerName, limitOrigin))
95+
get("/foo/2").accept(MediaType.APPLICATION_JSON).header(headerName, limitOrigin))
8296
.andExpect(status().isOk())
8397
.andExpect(content().json(ResultWrapper.blocked().toJsonString()));
8498

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/controller/TestController.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import org.springframework.web.bind.annotation.GetMapping;
2020
import org.springframework.web.bind.annotation.PathVariable;
21+
import org.springframework.web.bind.annotation.ResponseBody;
2122
import org.springframework.web.bind.annotation.RestController;
23+
import org.springframework.web.context.request.async.DeferredResult;
2224

2325
/**
2426
* @author kaizi2009
@@ -52,4 +54,16 @@ public String apiExclude(@PathVariable("id") Long id) {
5254
return "Exclude " + id;
5355
}
5456

57+
@GetMapping("/async")
58+
@ResponseBody
59+
public DeferredResult<String> distribute() throws Exception {
60+
DeferredResult<String> result = new DeferredResult<>();
61+
62+
Thread thread = new Thread(() -> result.setResult("async result."));
63+
thread.start();
64+
65+
Thread.yield();
66+
return result;
67+
}
68+
5569
}

‎sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/controller/WebMvcTestController.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@
1717

1818
import java.util.Random;
1919
import java.util.concurrent.TimeUnit;
20+
2021
import org.springframework.stereotype.Controller;
2122
import org.springframework.web.bind.annotation.GetMapping;
2223
import org.springframework.web.bind.annotation.PathVariable;
2324
import org.springframework.web.bind.annotation.ResponseBody;
25+
import org.springframework.web.context.request.async.DeferredResult;
2426
import org.springframework.web.servlet.ModelAndView;
2527

2628
/**
2729
* Test controller
30+
*
2831
* @author kaizi2009
2932
*/
3033
@Controller
@@ -57,14 +60,25 @@ public String apiExclude(@PathVariable("id") Long id) {
5760
doBusiness();
5861
return "Exclude " + id;
5962
}
60-
63+
6164
@GetMapping("/forward")
6265
public ModelAndView apiForward() {
6366
ModelAndView mav = new ModelAndView();
6467
mav.setViewName("hello");
6568
return mav;
6669
}
6770

71+
@GetMapping("/async")
72+
@ResponseBody
73+
public DeferredResult<String> distribute() throws Exception {
74+
DeferredResult<String> result = new DeferredResult<>(4000L);
75+
76+
Thread thread = new Thread(() -> result.setResult("async result"));
77+
thread.start();
78+
79+
return result;
80+
}
81+
6882
private void doBusiness() {
6983
Random random = new Random(1);
7084
try {

0 commit comments

Comments
 (0)