Pinned thread when selecting an instance from Arc about quarkus

[mkouba]

Hm, I thought that java.util.concurrent.locks.ReentrantLock is the recommended way to replace the synchronized blocks in order to avoid pinning 🤔.

CC @mariofusco @cescoffier @xstefank

[mkouba]

I investigated a little bit more this problem and, as correctly reported by @franz1981, I believe it is caused by this synchronization https://github.com/smallrye/smallrye-fault-tolerance/blame/63dce3532edbaa7c2b666671a5e7a1e0700d6521/implementation/core/src/main/java/io/smallrye/faulttolerance/core/apiimpl/LazyFaultTolerance.java#L41

In general I don't like that double check lazy initialization pattern, but I understand that sometimes it is necessary. I will try to replace it with an AtomicReference, but if you have any better suggestion please let me know. /cc @Ladicek @FroMage

Thanks for your investigation Mario!

[mariofusco]

See smallrye/smallrye-fault-tolerance#1035

[quarkus-bot]

/cc @Ladicek (arc), @manovotn (arc), @mkouba (arc)

[vsevel]

I believe so as well. I do not know if there is synchronized or a wait hidden in the stack. would we see it in this stacktrace?
I suppose so according to PinnedThreadPrinter.printStackTrace():

    private static void printStackTrace(List<LiveStackFrame> stack,
                                        PrintStream out,
                                        boolean printAll) {
        out.println(Thread.currentThread());
        for (LiveStackFrame frame : stack) {
            var ste = frame.toStackTraceElement();
            int monitorCount = frame.getMonitors().length;
            if (monitorCount > 0) {
                out.format("    %s <== monitors:%d%n", ste, monitorCount);
            } else if (frame.isNativeMethod() || printAll) {
                out.format("    %s%n", ste);
            }
        }
    }

[xstefank]

Most likely there is. ReentrantLock is working.

[vsevel]

Most likely there is.

do you know how to chase it?

[xstefank]

@vsevel I don't think there is a general way how to chase it. Can you provide me with a reproducer? Just to be sure there are no monitors in your business code? I can't reproduce it.

[mariofusco]

@vsevel I could also give a look at this, but I agree with @xstefank on the fact that we need a reproducer.

[vsevel]

it is unlikely that I will be able to give a reproducer. I am totally fine if at some point we close it with "cannot reproduce". I still wanted to create the issue so that we have a place to provide as much context as possible, and that is something that can be searched by others that experience the same issue.

[vsevel]

a bit more information. I looked at the app that was experiencing that particular issue, with that exact stacktrace.
it all happened on the same day. and in 2 occasions I looked at the error occurred just after startup.
and what is particularly disturbing is that in these 2 cases, the application experienced multiple exceptions and timeouts, and eventually killed shortly after because of the SmallRye Reactive Messaging - liveness check being down. one time it was because of org.jboss.resteasy.reactive.ClientWebApplicationException: Received: ', status code 401' when invoking ..., and the other time it was {"name":"SmallRye Reactive Messaging - liveness check","status":"DOWN","data":{"x":"[OK]","x":"[KO] - Multiple exceptions caught:\n\t[Exception 0] org.jboss.resteasy.reactive.ClientWebApplicationException: Received: 'Bad Gateway, status code 502' when invoking:.
I do not know what is causing the other.

[franz1981]

I see io.smallrye.faulttolerance.core.apiimpl.LazyFaultTolerance.call(LazyFaultTolerance.java:28) <== monitors:1

Which have a monitor there?

[mariofusco]

I investigated a little bit more this problem and, as correctly reported by @franz1981, I believe it is caused by this synchronization https://github.com/smallrye/smallrye-fault-tolerance/blame/63dce3532edbaa7c2b666671a5e7a1e0700d6521/implementation/core/src/main/java/io/smallrye/faulttolerance/core/apiimpl/LazyFaultTolerance.java#L41

In general I don't like that double check lazy initialization pattern, but I understand that sometimes it is necessary. I will try to replace it with an AtomicReference, but if you have any better suggestion please let me know. /cc @Ladicek @FroMage

[mariofusco]

This issue should have been fixed with the merging of this pull request smallrye/smallrye-fault-tolerance#1035