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

Virtual thread pinning on MainEventProcessor.ensureHostnameCache #3312

Open
pavelbelonosov opened this issue Apr 2, 2024 · 1 comment
Open
Milestone

Comments

@pavelbelonosov
Copy link

Problem Statement

We use Java 21 and Spring Boot 3.2.4 with enabled virtual threads in plain web application. First request after app init leads to thread pinning: io.sentry.MainEventProcessor.ensureHostnameCache(MainEventProcessor.java:192) <== monitors:1. Subsequent requests are fine.

Thread[#143,ForkJoinPool-1-worker-1,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.parkNanos(VirtualThread.java:631)
    java.base/java.lang.System$2.parkVirtualThread(System.java:2648)
    java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:67)
    java.base/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:267)
    java.base/java.util.concurrent.FutureTask.awaitDone(FutureTask.java:497)
    java.base/java.util.concurrent.FutureTask.get(FutureTask.java:203)
    io.sentry.HostnameCache.updateCache(HostnameCache.java:121)
    io.sentry.HostnameCache.<init>(HostnameCache.java:77)
    io.sentry.HostnameCache.<init>(HostnameCache.java:64)
    io.sentry.HostnameCache.<init>(HostnameCache.java:58)
    io.sentry.HostnameCache.getInstance(HostnameCache.java:52)
    io.sentry.MainEventProcessor.ensureHostnameCache(MainEventProcessor.java:192) <== monitors:1
    io.sentry.MainEventProcessor.setServerName(MainEventProcessor.java:181)
    io.sentry.MainEventProcessor.processNonCachedEvent(MainEventProcessor.java:132)
    io.sentry.MainEventProcessor.process(MainEventProcessor.java:146)
    io.sentry.SentryClient.processTransaction(SentryClient.java:416)
    io.sentry.SentryClient.captureTransaction(SentryClient.java:661)
    io.sentry.Hub.captureTransaction(Hub.java:700)
    io.sentry.SentryTracer.finish(SentryTracer.java:264)
    io.sentry.SentryTracer.finish(SentryTracer.java:565)
    io.sentry.SentryTracer.finish(SentryTracer.java:558)
    io.sentry.SentryTracer.finish(SentryTracer.java:553)
    io.sentry.spring.jakarta.tracing.SentryTracingFilter.doFilterWithTransaction(SentryTracingFilter.java:127)
    io.sentry.spring.jakarta.tracing.SentryTracingFilter.doFilterInternal(SentryTracingFilter.java:87)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    io.sentry.spring.jakarta.SentrySpringFilter.doFilterInternal(SentrySpringFilter.java:71)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:167)
    org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90)
    org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:482)
    org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:115)
    org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
    org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:344)
    org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:391)
    org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
    org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:896)
    org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1744)
    org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
    java.base/java.lang.VirtualThread.run(VirtualThread.java:311)

This behavior is observed in Sentry 7.6.0 and 6.26.0

Solution Brainstorm

Would be nice if Sentry became Loom-friendly and replace synchronized methods to ReentrantLock for virtual thread support.

From this

private void ensureHostnameCache() {
    if (hostnameCache == null) {
      synchronized (this) {
        if (hostnameCache == null) {
          hostnameCache = HostnameCache.getInstance();
        }
      }
    }
  }

To this:

    private final ReentrantLock lock = new ReentrantLock();

    private void ensureHostnameCache() {
        if (hostnameCache == null) {
            lock.lock();
            try {
                if (hostnameCache == null) {
                    hostnameCache = HostnameCache.getInstance();
                }
            } finally {
                lock.unlock();
            }
        }
    }
@adinauer
Copy link
Member

adinauer commented Apr 5, 2024

Hey @pavelbelonosov thanks for opening this issue. Sounds like a straight forward replacement. Maybe we can fit this into the next major which we're starting to work on - no promises though. We're currently changing a lot of things on the SDK internally, so we can't do it right away or we'd have lots of merge conflicts. ReentrantLock seems to be available for old versions of Android as well.

This class in pgjdbc seems like an interesting approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: Needs Discussion
Development

No branches or pull requests

3 participants