Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit synchronized blocks involving blocking operations #2690

Closed
giger85 opened this issue Aug 23, 2023 · 2 comments
Closed

Revisit synchronized blocks involving blocking operations #2690

giger85 opened this issue Aug 23, 2023 · 2 comments
Assignees
Labels
type: task A general task

Comments

@giger85
Copy link

giger85 commented Aug 23, 2023

Version

  • Spring Boot 3.2.0-M1
  • Spring Data Redis 3.2.0-M1 & lettuce-core 6.2.5.RELEASE
  • Java 21 (Temurin-21+35-202308102344)

Issue

Detects virtual thread parking & pinning when LettuceConnectionFactory.getSharedConnection() is called.
Stack trace is below. (need to be apply -Djdk.tracePinnedThreads=full)

Thread[#116,ForkJoinPool-1-worker-2,5,CarrierThreads]
    java.base/java.lang.VirtualThread$VThreadContinuation.onPinned(VirtualThread.java:185)
    java.base/jdk.internal.vm.Continuation.onPinned0(Continuation.java:393)
    java.base/java.lang.VirtualThread.park(VirtualThread.java:592)
    java.base/java.lang.System$2.parkVirtualThread(System.java:2639)
    java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:54)
    java.base/java.util.concurrent.locks.LockSupport.park(LockSupport.java:219)
    java.base/java.util.concurrent.CompletableFuture$Signaller.block(CompletableFuture.java:1864)
    java.base/java.util.concurrent.ForkJoinPool.unmanagedBlock(ForkJoinPool.java:3780)
    java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3725)
    java.base/java.util.concurrent.CompletableFuture.waitingGet(CompletableFuture.java:1898)
    java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2072)
    io.lettuce.core.DefaultConnectionFuture.get(DefaultConnectionFuture.java:69)
    io.lettuce.core.AbstractRedisClient.getConnection(AbstractRedisClient.java:345)
    io.lettuce.core.RedisClient.connect(RedisClient.java:216)
    org.springframework.data.redis.connection.lettuce.StandaloneConnectionProvider.lambda$getConnection$1(StandaloneConnectionProvider.java:112)
    java.base/java.util.Optional.orElseGet(Optional.java:364)
    org.springframework.data.redis.connection.lettuce.StandaloneConnectionProvider.getConnection(StandaloneConnectionProvider.java:112)
    org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$ExceptionTranslatingConnectionProvider.getConnection(LettuceConnectionFactory.java:1626)
    org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$SharedConnection.getNativeConnection(LettuceConnectionFactory.java:1453)
    org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$SharedConnection.getConnection(LettuceConnectionFactory.java:1436) <== monitors:1
    org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory.getSharedConnection(LettuceConnectionFactory.java:1127)
    org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory.getConnection(LettuceConnectionFactory.java:465)
...

Do you have any plan that avoid thread pinning? (ex. Use ReentrantLock)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 23, 2023
@mp911de mp911de added this to the 3.2 M3 (2023.1.0) milestone Aug 23, 2023
@mp911de mp911de added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 23, 2023
@mp911de mp911de changed the title Detect thread pinning when LettuceConnectionFactory.getSharedConnection() is called Revisit synchronized blocks involving blocking operations Aug 23, 2023
@mp911de
Copy link
Member

mp911de commented Aug 23, 2023

Thanks a lot for letting us know. It makes indeed sense to revisit our synchronized blocks to avoid platform thread pinning.

@mp911de mp911de self-assigned this Aug 23, 2023
@giger85
Copy link
Author

giger85 commented Aug 23, 2023

@mp911de
Thanks for your reply.
I will wait next milestone version. (It might be 3.2 M3)

mp911de added a commit that referenced this issue Aug 23, 2023
To avoid thread pinning on virtual thread arrangements we now use ReentrantLock instead of a synchronized block.

Closes #2690
mp911de added a commit that referenced this issue Aug 23, 2023
Add missing Override annotations.

See #2690
jxblum added a commit to jxblum/spring-data-redis that referenced this issue Sep 13, 2023
jxblum added a commit to jxblum/spring-data-redis that referenced this issue Sep 13, 2023
jxblum pushed a commit to jxblum/spring-data-redis that referenced this issue Sep 13, 2023
Add missing Override annotations.

See spring-projects#2690
jxblum added a commit to jxblum/spring-data-redis that referenced this issue Sep 13, 2023
@jxblum jxblum closed this as completed in 7fbfd5d Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
3 participants