Spring Boot DeferredResult Aspect makes CPU go sky high

Question

We are using Spring Boot to develop our servies. We chose to do it in an async way and we are faced with the following problem: We have the following aspect on top of all our async rest resources:

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.reflect.MethodSignature;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Component;
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
import org.springframework.web.context.request.async.DeferredResult;


@Aspect
@Component
public class TerminatedUsersAspect {

    private static final Logger LOGGER = LoggerFactory.getLogger("[TERMINATED_USERS]");
    public static final ThreadLocal<String> ID_LOCAL = new ThreadLocal<>();

    @Autowired
    private UserRepository userRepo;

    @Autowired
    private UserService userService;

    @Autowired
    private ExecutorService executorService;

    @SuppressWarnings("unchecked")
    @Around("within(com.test..*) && @annotation(authorization)")
    public Object checkForId(ProceedingJoinPoint joinPoint, Authorization authorization) throws Throwable {

        final MethodInvocationProceedingJoinPoint mJoinPoint = (MethodInvocationProceedingJoinPoint) joinPoint;
        final MethodSignature signature = (MethodSignature) mJoinPoint.getSignature();
        final DeferredResult<Object> ret = new DeferredResult<>(60000L);
        final String id = ID_LOCAL.get();

        if (signature.getReturnType().isAssignableFrom(DeferredResult.class) && (id != null)) {

            userRepo.getAccountStatus(id).thenAcceptAsync(status -> {
                boolean accountValid = userService.isAccountValid(status, true);

                if (!accountValid) {
                    LOGGER.debug("AccountId: {} is not valid. Rejecting with 403", id);
                    ret.setErrorResult(new ResponseEntity<String>("Invalid account.", HttpStatus.FORBIDDEN));
                    return;
                }

                try {

                    final DeferredResult<Object> funcRet = (DeferredResult<Object>) joinPoint.proceed();

                    funcRet.setResultHandler(r -> {
                        ret.setResult(r);
                    });

                    funcRet.onTimeout(() -> {
                        ret.setResult(new AsyncRequestTimeoutException());
                    });

                } catch (Throwable e) {
                    ret.setErrorResult(ret);
                }

            }, executorService).exceptionally(ex -> {
                ret.setErrorResult(ex);
                return null;
            });

            return ret;
        }

        return joinPoint.proceed();
    }

}

Our embedded server in the application is undertow. The problem arises in time. It seems that after almost one day because of this aspect the CPUs evetually end up spiking 100% red. I debugged the code and seems to be fine from my point of view, but maybe I am missing something ? Any ideas would be welcomed. Thanks, C.

Answer 1

First thing that strikes me is that you invoke joinPoint.proceed() twice in your code. First one when you cast it to DeferredResult and the second one when you return from method. You should rewrite it so it will be executed only once. You may consider using @Around annotation with an 'execution' (https://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html#aop-pointcuts-examples) instead of 'within' as it allows you to specify return type. In that case you won't need that if based on method return type.

Having that said, I'm not sure if this will resolve your problem. I'm lacking a context there and I don't know what you are trying to achieve. Possibly there is a simpler way to do that as usage of deferred result in that context is a bit strange for me. Besides, what particularly was causing that 100% resources usage? Were there some threads running infinitely or some HTTP connections were hanging?

Answer 2

How about this little refactoring? It is typesafe, does not need reflection and only calls proceed() once.

Furthermore, I thought that in your code ret.setErrorResult(ret) did not make much sense and I replaced it by setErrorResult(e), setting the actual execption as the error result.

I also hope you will forgive me for renaming a few variables. It helped me better to understand what was going on. I am still not 100% sure because I know nothing about Spring (but much about AspectJ).

package de.scrum_master.aspect;

import java.util.concurrent.ExecutorService;

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Component;
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
import org.springframework.web.context.request.async.DeferredResult;

@Aspect
@Component
public class TerminatedUsersAspect {

  private static final Logger LOGGER = LoggerFactory.getLogger("[TERMINATED_USERS]");
  public static final ThreadLocal<String> ID_LOCAL = new ThreadLocal<>();

  @Autowired private UserRepository userRepo;
  @Autowired private UserService userService;
  @Autowired private ExecutorService executorService;

  @Around(
    "within(com.test..*) && @annotation(authorization) && " +
    "execution(org.springframework.web.context.request.async.DeferredResult *(..))"
  )
  public DeferredResult<Object> checkForId(ProceedingJoinPoint joinPoint, Authorization authorization) throws Throwable {

    final DeferredResult<Object> aspectResult = new DeferredResult<>(60000L);
    final String id = ID_LOCAL.get();

    userRepo
      .getAccountStatus(id)
      .thenAcceptAsync(status -> {
          boolean accountValid = userService.isAccountValid(status, true);
          if (!accountValid) {
            LOGGER.debug("AccountId: {} is not valid. Rejecting with 403", id);
            aspectResult.setErrorResult(new ResponseEntity<String>("Invalid account.", HttpStatus.FORBIDDEN));
            return;
          }
          try {
            @SuppressWarnings("unchecked")
            final DeferredResult<Object> originalResult = (DeferredResult<Object>) joinPoint.proceed();
            originalResult.setResultHandler(r -> { aspectResult.setResult(r); });
            originalResult.onTimeout(() -> { aspectResult.setResult(new AsyncRequestTimeoutException()); });
          } catch (Throwable e) {
            aspectResult.setErrorResult(e);
          }
        },
        executorService
      )
      .exceptionally(ex -> {
          aspectResult.setErrorResult(ex);
          return null;
        }
      );

    return aspectResult;
  }
}