Recursive call in AsyncConnectionProvider

[pauldthomson]

Bug Report

We have stumbled on a recursive update error in the AsyncConnectionProvider when using Redis to back a Spring Session used in Spring Security.

Current Behavior

When a cold instance of our app receives new requests with authenticated session cookies, the session lookup lands in AsyncConnectionProvider.getSynchronizer via the PooledClusterConnectionProvider.

As this is a freshly started app instance the connections map in AsyncConnectionProvider is empty, so we call into the connections.computeIfAbsent mapping function. That mapping function ultimately leads to a Mono.subscribe that ends up invoking SecurityContextHolder.getContext() which then triggers another session lookup (while the mapping function is still in flight).

This leads to a recursive update exception being thrown from ConcurrentHashMap.putIfAbsent.

Stack trace

 - Reactor/context bridge:
 ```text
   io.lettuce.core.AbstractRedisClient.initializeChannelAsync(AbstractRedisClient.java:411)
   -> reactor.core.publisher.Mono.subscribe(Mono.java:4450)
   -> reactor.core.publisher.Mono.subscribe(Mono.java:4478)
   -> reactor.core.publisher.Mono.subscribe(Mono.java:4542)
   -> reactor.core.publisher.Mono.subscribeWith(Mono.java:4641)
   -> reactor.core.publisher.Mono.subscribe(Mono.java:4576)
   -> reactor.core.publisher.MonoCallable.subscribe(MonoCallable.java:48)
   -> reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onSubscribe(MonoPeekTerminal.java:152)
   -> reactor.core.publisher.FluxPeekFuseable$PeekFuseableSubscriber.onSubscribe(FluxPeekFuseable.java:178)
   -> org.springframework.security.config.annotation.web.configuration.SecurityReactorContextConfiguration$SecurityReactorContextSubscriber.onSubscribe(SecurityReactorContextConfiguration.java:181)
   -> reactor.core.publisher.FluxContextWriteRestoringThreadLocalsFuseable$FuseableContextWriteRestoringThreadLocalsSubscriber.onSubscribe(FluxContextWriteRestoringThreadLocalsFuseable.java:102)
   -> reactor.core.publisher.ContextPropagation.setThreadLocals(ContextPropagation.java:84)
   -> io.micrometer.context.DefaultContextSnapshotFactory.setThreadLocalsFrom(DefaultContextSnapshotFactory.java:109)
   -> io.micrometer.context.DefaultContextSnapshotFactory.setAllThreadLocalsFrom(DefaultContextSnapshotFactory.java:128)
   -> io.micrometer.context.DefaultContextSnapshot.clearThreadLocal(DefaultContextSnapshot.java:110)
   -> org.springframework.security.core.context.SecurityContextHolderThreadLocalAccessor.getValue(SecurityContextHolderThreadLocalAccessor.java:37)
   -> org.springframework.security.core.context.SecurityContextHolderThreadLocalAccessor.getValue(SecurityContextHolderThreadLocalAccessor.java:46)
   -> org.springframework.security.core.context.SecurityContextHolder.getContext(SecurityContextHolder.java:125)

• Spring Security / Spring Session re-entry:

  -> org.springframework.security.core.context.ThreadLocalSecurityContextHolderStrategy.getContext(ThreadLocalSecurityContextHolderStrategy.java:43)
  -> org.springframework.security.core.context.ThreadLocalSecurityContextHolderStrategy.lambda$setDeferredContext$2(ThreadLocalSecurityContextHolderStrategy.java:67)
  -> org.springframework.security.web.context.DelegatingSecurityContextRepository$DelegatingDeferredSecurityContext.get(DelegatingSecurityContextRepository.java:96)
  -> org.springframework.security.web.context.DelegatingSecurityContextRepository$DelegatingDeferredSecurityContext.get(DelegatingSecurityContextRepository.java:109)
  -> org.springframework.security.web.context.SupplierDeferredSecurityContext.get(SupplierDeferredSecurityContext.java:33)
  -> org.springframework.security.web.context.SupplierDeferredSecurityContext.get(SupplierDeferredSecurityContext.java:52)
  -> org.springframework.security.web.context.SupplierDeferredSecurityContext.init(SupplierDeferredSecurityContext.java:67)
  -> org.springframework.security.web.context.HttpSessionSecurityContextRepository.lambda$loadDeferredContext$0(HttpSessionSecurityContextRepository.java:145)
  -> jakarta.servlet.http.HttpServletRequestWrapper.getSession(HttpServletRequestWrapper.java:221)
  -> org.springframework.session.web.http.SessionRepositoryFilter$SessionRepositoryRequestWrapper.getSession(SessionRepositoryFilter.java:193)
  -> org.springframework.session.web.http.SessionRepositoryFilter$SessionRepositoryRequestWrapper.getSession(SessionRepositoryFilter.java:286)
  -> org.springframework.session.web.http.SessionRepositoryFilter$SessionRepositoryRequestWrapper.getRequestedSession(SessionRepositoryFilter.java:352)
  -> org.springframework.session.data.redis.RedisSessionRepository.findById(RedisSessionRepository.java:45)
  -> org.springframework.session.data.redis.RedisSessionRepository.findById(RedisSessionRepository.java:138)
  -> org.springframework.data.redis.core.DefaultHashOperations.entries(DefaultHashOperations.java:323)
  -> org.springframework.data.redis.connection.lettuce.LettuceHashCommands.hGetAll(LettuceHashCommands.java:110)
  -> io.lettuce.core.AbstractRedisAsyncCommands.hgetall(AbstractRedisAsyncCommands.java:1272)
  -> io.lettuce.core.cluster.StatefulRedisClusterConnectionImpl.dispatch(StatefulRedisClusterConnectionImpl.java:242)
  -> io.lettuce.core.cluster.ClusterDistributionChannelWriter.doWrite(ClusterDistributionChannelWriter.java:270)

• Representative outermost-to-innermost summary:

  Jetty request thread
  -> Spring Security filter chain
  -> RememberMeAuthenticationFilter
  -> SecurityContextHolder.getContext()
  -> HttpSessionSecurityContextRepository.loadDeferredContext(...)
  -> request.getSession(false)
  -> SessionRepositoryRequestWrapper.getRequestedSession()
  -> RedisSessionRepository.findById(sessionId)
  -> DefaultHashOperations.entries(sessionKey)
  -> LettuceHashCommands.hGetAll(sessionKey)
  -> StatefulRedisClusterConnectionImpl.dispatch(...)
  -> ClusterDistributionChannelWriter.doWrite(...)
  -> PooledClusterConnectionProvider.getConnectionAsync(slot/key)
  -> AsyncConnectionProvider.getSynchronizer(outer key)
  -> ConcurrentHashMap.computeIfAbsent(outer key, mapping)
  -> DefaultClusterNodeConnectionFactory.apply(...)
  -> RedisClusterClient.connectToNodeAsync(...)
  -> RedisClusterClient.connectStatefulAsync(...)
  -> AbstractRedisClient.initializeChannelAsync(...)
  -> Reactor Mono.subscribe(...)
  -> Reactor / Micrometer / Spring Security context restoration
  -> SecurityContextHolder.getContext()
  -> SessionRepositoryRequestWrapper.getRequestedSession()
  -> RedisSessionRepository.findById(sessionId)
  -> LettuceHashCommands.hGetAll(sessionKey)
  -> ClusterDistributionChannelWriter.doWrite(...)
  -> PooledClusterConnectionProvider.getConnectionAsync(slot/key)
  -> AsyncConnectionProvider.getSynchronizer(same key)
  -> ConcurrentHashMap.computeIfAbsent(same key, ...)
  -> IllegalStateException: Recursive update

Input Code

Input Code

// your code here;

Expected behavior/code

No recursive update to happen.

Environment

• Lettuce version(s): 6.6.0.RELEASE
• Redis version: N/A

Possible Solution

I have a draft PR that effectively changes the putIfAbsent to take a placeholder Sync. That Sync triggers the existing connectionFactoy.apply that was happening inside the putIfAbsent mapping function but now does it outside it, meaning that any recursive calls to putIfAbsent just receive the placeholder Sync until it completes, at which point the connection is updated on the Sync object

Additional context

The recursive loop was also found in the RedisClusterClient.connectToNodeAsync method when the connectionFuture throws an exception, as this calls connection.closeAsync() which loops back to the channel writer and ultimately back to the putIfAbsent call. We didn't hit this in our case but found that path whilst trying to trace what was going on

[a-TODO-rov]

Hey @pauldthomson
Thanks for reporting this.
If you already have a PR ready, would you be able to share it for review ?
Also if you can share some reproducible example, it would be very helpful.

[pauldthomson]

Thanks @a-TODO-rov, I'll clean up the PR and link it here. Also just working on a minimal repro of the issue, will share soon.