26

According to this answer thread local variable when we use thread local we should clear all variable in thread pool environment.

So basically I just want to confirm that when we are using MDC (Mapped diagnostic context) we also should clear MDC to aware memory leaks, is that true ?

So for instance :

@Configuration
public class WebConfig implements WebMvcConfigurer {
    public class HttpInterceptor implements HandlerInterceptor {
        @Override
        public boolean preHandle(final HttpServletRequest request,
                                 final HttpServletResponse response,
                                 final Object handler) {
            MDC.put(SESSION_ID, session_id);
        {

        @Override
        public void postHandle(final HttpServletRequest request,
                               final HttpServletResponse response,
                               final Object handler,
                               final ModelAndView modelAndView) {
           MDC.clear(); //WE SHOULD CLEAR MDC.... if not memory leaks ?
        }
    }

    @Override
    public void addInterceptors(InterceptorRegistry registry) {
        registry.addInterceptor(new MdcHandlerInterceptor());
    }
}
Dan Oak
  • 704
  • 1
  • 7
  • 26
Łukasz Woźniczka
  • 1,625
  • 3
  • 28
  • 51

4 Answers4

24

Not for memory leaks, but to prevent retaining information between requests. You don't want your first request putting foo and your second request putting bar, ending up with foo bar instead of just bar.

Of course if you're always filling up only the same exact values (like remote IP, etc.), this couldn't happen, but better safe than sorry. You don't want to log bad information.

Note: the information that you put in one request is not always propagated to next requests, because they can be executed on other threads even for the same endpoint. That's why the issue may be overlooked as it doesn't reproduce reliably, especially in cases where you're overwriting most of the values.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
0

You can write a test code as below.

    public static void request(ThrowingRunnable runnable, String para)  {
            CompletableFuture.runAsync(() -> {
                MDC.put("var"+ para, "value"+ para);
                try {
                    runnable.runThrows();
                } catch (Exception e) {
                    log.warn(e.getMessage(), e);
                }finally {
                    // get MDC variables
                    String st = MDC.getCopyOfContextMap().entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).reduce((a, b) -> a + "," + b).orElse("");
                    log.info(st);
                }
            });
    }

    public static void main(String[] args) throws InterruptedException {
        for (int i = 0; i < 100; i++) {
            request(() -> {
            }, i + "");
        }
        Thread.sleep(10000);
    }
Jess Chen
  • 3,136
  • 1
  • 26
  • 35
0

Whether you should clear the MDC or not after an HTTP request depends on your specific use case. If you are using the MDC to store information that is specific to a particular request and should not be carried over to subsequent requests, then it might be a good idea to clear the MDC after the request has been processed. This can help to ensure that the MDC does not contain any stale or irrelevant information.

-2

MDC is thread-based so it is not possible that Information is shared between threads... Only if in the same thread somebody uses the MDC-Values again it could have effects... And when the thread ends, the ThreadLocalMap of MDC will disapear... I guess...

  • 3
    You don't need to answer ancient questions that already have answers, and you especially shouldn't answer with *guesses*. If you feel my answer is missing something, do tell and I'll elaborate on it. – Kayaman Aug 06 '20 at 12:14
  • 4
    The danger is that requests are handled by a thread pool where threads are reused and retain their previous values. So it's better to explicitly clear the MDC so the thread which handled the request can be reused. – Davio Jan 18 '21 at 07:48