search

jakartaee / mail-api

Jakarta Mail Specification project
https://jakartaee.github.io/mail-api
Other
244 stars 101 forks source link

`IdleManager.watch` freezes forever #732

Closed boris-petrov closed 3 months ago

boris-petrov commented 3 months ago

Describe the bug IdleManager.watch freezes forever

To Reproduce I don't have a reproduction, I just see it happening in production now and then. Here's the stack trace:

      java.base/jdk.internal.misc.Unsafe.park(Native Method)
      java.base/java.lang.VirtualThread.parkOnCarrierThread(VirtualThread.java:675)
      java.base/java.lang.VirtualThread.park(VirtualThread.java:607)
      java.base/java.lang.System$2.parkVirtualThread(System.java:2648)
      java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:54)
      java.base/java.util.concurrent.locks.LockSupport.park(LockSupport.java:369)
      java.base/sun.nio.ch.Poller.poll(Poller.java:178)
      java.base/sun.nio.ch.Poller.poll(Poller.java:137)
      java.base/sun.nio.ch.SelChImpl.park(SelChImpl.java:88)
      java.base/sun.nio.ch.SelChImpl.park(SelChImpl.java:116)
      java.base/sun.nio.ch.SocketChannelImpl.blockingRead(SocketChannelImpl.java:1425)
      java.base/sun.nio.ch.SocketInputStream.implRead(SocketInputStream.java:70)
      java.base/sun.nio.ch.SocketInputStream.read(SocketInputStream.java:78)
      com.sun.mail.util.TraceInputStream.read(TraceInputStream.java:102)
      java.base/java.io.BufferedInputStream.fill(BufferedInputStream.java:291)
      java.base/java.io.BufferedInputStream.implRead(BufferedInputStream.java:325)
      java.base/java.io.BufferedInputStream.read(BufferedInputStream.java:312)
      com.sun.mail.iap.ResponseInputStream.readResponse(ResponseInputStream.java:79)
      com.sun.mail.iap.Response.<init>(Response.java:109)
      com.sun.mail.imap.protocol.IMAPResponse.<init>(IMAPResponse.java:36)
      com.sun.mail.imap.protocol.IMAPProtocol.readResponse(IMAPProtocol.java:388)
      com.sun.mail.imap.protocol.IMAPProtocol.idleStart(IMAPProtocol.java:3152)
      com.sun.mail.imap.IMAPFolder$19.doCommand(IMAPFolder.java:3186)
      com.sun.mail.imap.IMAPFolder.doProtocolCommand(IMAPFolder.java:3888)
      com.sun.mail.imap.IMAPFolder.doOptionalCommand(IMAPFolder.java:3852)
      com.sun.mail.imap.IMAPFolder.startIdle(IMAPFolder.java:3175)
      com.sun.mail.imap.IdleManager.watch(IdleManager.java:193)
      ...

This freezes forever. I'm not sure why and what I can do about it.

Expected behavior watch to not freeze.

Screenshots N/A

Desktop (please complete the following information): N/A

Smartphone (please complete the following information): N/A

Mail server:

Additional context The library used is com.sun.mail:jakarta.mail:1.6.7.

I don't understand why that would block. I see exactly 30 minutes after that in the courier-imap logs: imapd[1158]: TIMEOUT - which is, I believe, the default IDLE timeout of the server.

Is that an issue in the mail server or in Java Mail? What can I do about that? I've set mail.imap.connectiontimeout and mail.imap.writetimeout to 10_000. I can't set mail.imap.timeout as that seems to cause all kinds of deadlocks but that's another story.

jmehrens commented 3 months ago

From the stacktrace you are using virtual threads. JavaMail/JakartaMail uses synchronized blocks in the code which will pin the carrier thread. My understanding is if enough carrier threads get pinned the worker pool will get exhausted and there will be no workers to run the async tasks. I would make sure that IdleManager.watch is not called from a virtual thread and re-test this.

There was an old abandon branch that Bill was working on but never merged: https://github.com/jakartaee/mail-api/issues/404 Might be something to try. My understanding is you will always have to handle errors from the server to retry or abort.

boris-petrov commented 3 months ago

@jmehrens thank you for the response. I think you are right that in this case this is what happens. I should have seen it but I was searching for pinned threads in the thread dump but for some reason the dump from jcmd didn't show correctly blocked threads on synchronized. So I got confused. Thank you for helping me find it! :) I'll close the issue.

jmehrens commented 3 months ago

IdleManager is a bit special in that it might be worth switching to ReentrantLock. Looks trivial to do and class is marked experimental so compatibility change is not a big deal. Out of all of the classes here this one might good candidate to make virtual thread friendly.

boris-petrov commented 3 months ago

Well, users of the class don't/shouldn't care whether it uses synchronized or locks underneath so it wouldn't even be a compatibility-change. But I'm not sure whether IdleManager itself doesn't depend on IMAPFolder using synchronized and so it has to use it too...

jmehrens commented 3 months ago

I was just looking at code idlemanager code. You are right I need to look at the whole stack trace as there are other locks later on.

I was thinking this might have been a candidate to fix but if it starts to get outside of idlemanager then this will face the same issues as https://github.com/eclipse-ee4j/angus-mail/issues/128 and I'll have to leave this this closed.

boris-petrov commented 3 months ago

In the end the whole thing might not be needed. The Loom guys already have a prototype for a fix of the synchronized thread-pinning issue so you could just wait and the issue will go away by itself. :) Hopefully for JDK 24. Java Mail is definitely not the only project with such a "problem" so no need to worry about it. It's up to us, the users, to figure it out for now - we're the ones using a cutting-edge feature after all. :)

P.S. Missed a "not" above. :)