Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8319090

StackOverflow from ThreadLocal.set might shadow the root cause

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: P4 P4
    • tbd
    • 11, 17, 21, 22
    • core-libs
    • None

      One of the cases for ThreadLocal is to pass context via try-finally.

      For instance:
      https://github.com/spring-projects/spring-framework/blob/8770544769367b780b3667b832920c78d4a87611/spring-aop/src/main/java/org/springframework/aop/interceptor/ExposeInvocationInterceptor.java#L94-L101

      private static final ThreadLocal<MethodInvocation> invocation =
      new NamedThreadLocal<>("Current AOP method invocation");

      public Object invoke(MethodInvocation mi) throws Throwable {
      MethodInvocation oldInvocation = invocation.get();
      invocation.set(mi);
      try {
      return mi.proceed();
      }
      finally {
      invocation.set(oldInvocation);
      }
      }

      The code assumes it will "always" restore the original invocation value, however, it fails to do so in the case of StackOverflowError in the "finally" block.

      There's a similar case in OpenJDK as well: https://github.com/openjdk/jdk/blob/3934127b087ade1c1286008df3497ca6d84778a5/src/jdk.dynalink/share/classes/jdk/dynalink/LinkerServicesImpl.java#L161-L172

      I suggest adding @ReservedStackAccess to ThreadLocal#set, setInitialValue, remove so the code that uses finally { threadLocal.set(...) } doesn't fail with StackOverflowError

      The case has been raised in https://mail.openjdk.org/pipermail/core-libs-dev/2023-September/112702.html , and I think adding @RSA is a reasonable improvement targeting a common case which is otherwise hard to diagnose.

            vklang Viktor Klang
            vsitnikov Vladimir Sitnikov
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: