ServerInterceptor code doesn't get invoked for one of the gRPC request on calling multiple requests

[mismaiel68]

The context We are facing one issue with the server interceptor that the intermittent requests doesn't invoke the interceptor code when continuously calling multiple requests at the same time. This issue is affecting the whole flow since we are setting a really important info regarding the context in the interceptCall function of the ServerInterceptor.

What do you wish to achieve? interceptCall must gets executed before actual gRPC service even in case spam requests

The bug interceptCall is not getting invoked for each request

Steps to Reproduce Continuously invoke multiple gRPC requests at the same time and will notice that at some point intermittent request didn't go through the interceptor

Which versions do you use?

• Spring (boot): 2.7.10
• grpc-java: 1.54.0
• grpc-spring-boot-starter: 2.14.0.RELEASE
• java: version + architecture (64bit?) 17

[ST-DDT]

Steps to Reproduce

Please provide actual code to reproduce/analyze this.

Our authentication code is interceptor based, and I literally run a ton of heavily parallel requests to debug a security issue with them. This issue actually reminds me of that old bug:

• 126

Where do you store the data for your context? Please note that a normal ThreadLocal is not the correct place and will cause the issues described above if no workarounds are in place.

[mismaiel68]

Here is the code snippet:

@Override
  public <ReqT, RespT> Listener<ReqT> interceptCall(
      ServerCall<ReqT, RespT> call, Metadata headers, ServerCallHandler<ReqT, RespT> next) {
    Optional<String> identifierOpt =
        Optional.ofNullable(
            headers.get(Metadata.Key.of("X-Identifier", Metadata.ASCII_STRING_MARSHALLER)));
    String identifier = identifierOpt.get();
    ContextHolder.setContext(identifier);
    return next.startCall(call, headers);
  }

And, yes, we are using ThreadLocal to store the context data.

private static final ThreadLocal<TenantVO> contextHolder = new ThreadLocal<>();

We try to read/write the context data from/in the ThreadLocal using static set/get function of our ContextHolder class as mentioned in the snippet. So, our approach is to set the context data at the time of request interception and to read the context later in the gRPC service if needed. We are assuming that the execution after the startCall is being done by the same thread.

is ThreadLocal a problem here?

[ST-DDT]

is ThreadLocal a problem here?

Yes, because in concurrent environments the Thread is likely to switch between the interceptor and the actual target method multiple times. This varies between unary and streaming calls, but there are the following steps to each call: startCall, onReady, onMessage, onHalfClose, onComplete/onCancel, each potentially on a different thread; startCall just builds the road for the actual request. In light load scenarios, this is less likely to occur since there are fewer Threads involved (CachedThreadPool?). So in addition to the failures, you also have (more) cases where you got the wrong identifier/an identifier from a different request in the controller/service.

There are two recommended solutions to this.

• Attach the data to the GrpcContext
• Or use a @Scope("grpcRequest") bean

Does this help you?

[mismaiel68]

Thanks for the really quick reply.

what if we override onReady, onMessage, etc. and to set/unset the the context data like following:

@Override public void onMessage(ReqT message) { try { ContextHolder.setContext(this.identifier); super.onMessage(message); } finally { ContextHolder.clearContext(); } }

this will also solve the issue. right?

the same way you guys did in This PR

thanks again for your replies as it really helped in understanding the problem we are facing.

[ST-DDT]

this will also solve the issue. right?

Yes.

Dont forget this part: https://github.com/yidongnan/grpc-spring-boot-starter/pull/126/files#diff-6cd291d5e80f3f90a5a4ffffa7994c701122571ae95d18690e941ee52a7ddde8R78

[mismaiel68]

I did this change and it fixed the issue.

But there is a slightly performance degrade after doing this change which might be because we are setting / unsetting the context on all of the handlers. I would consider adding the change which you suggested above.

Thanks again for your help.

[ST-DDT]

If you don't need the value all the time/in other interceptors, you might skip everything except for startCall and onMessage, because that are the only parts that usually end up in the controller methods.