-
Notifications
You must be signed in to change notification settings - Fork 168
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
Potential deadlock with Jetty #19938
Comments
The big question on my mind is which thread holds the lock that the semaphore is waiting for? Thread dumps do typically also include information on locks held by each thread but that information doesn't seem to be there in the dumps you shared. It might be useful if you could somehow find a way of getting a thread dump that includes that data. |
This thread dump is a custom one produced by our code. I can try to come up with a better one. In this case, however, the thread waits on a condition, not a lock, so no other thread is "holding" the lock. The blocked thread is waiting in BlockingContentProducer.nextChunk until content is available. From what I see, the semaphore should eventually be released in BlockingContentProducer.onContentAvailable when new content is available, which should be called from HttpInput.run. I'm not really seeing the deadlock here, but I find it suspicious that some threads wait on the VaadinSession and another holds it while blocking for some other reason. I also thought about reporting this to the Jetty team, but since the locks on the VaadinSession seem to be involved, I figured you could have a more complete picture. |
I assume that one of those two threads waiting for the session lock would eventually do something that would allow the |
@sbordet from the Jetty team suggests not to perform a blocking read (or write) while holding a VaadinSession lock: I'm not sure whether that would be compatible with how syncIds are checked and incremented, but you should probably at least investigate this further. |
If my hypothesis about TCP head-of-line blocking holds true, then I don't see any other way around it than changing our code to not read while holding the session lock. Is see two potential ways of achieving that:
There's also a risk that we'd have to do something similar for writing the response but I'm not 100% sure about that. |
Is this related to #18077 ? |
@TatuLund I don't think so. I just tried to shutdown the server (we use Jetty, not Tomcat) with Atmosphere warnings enabled, but I didn't see anything. |
Looks like this could actually be a Jetty issue (jetty/jetty.project#12272 (comment)). Let's wait for Jetty project people to work that ticket to a conclusion before attempting to fix something in Vaadin. |
@tepi, eventually it was a Jetty issue for that very specific corner case. However, in general, what discussed with @Legioth above holds valid: Vaadin should not hold the session lock while calling blocking code (either blocking reads or blocking writes). Even with the Jetty fix, the blocking code may be woken up after a long idle timeout, and meanwhile the session lock would cause other threads to block, causing a cascading effect where all threads could block and lockup the server. |
Fair enough. Jetty fix looks good. I'll start looking into how we could refactor the blocking reads and writes out of session lock being held. |
Blocking calls for |
Description of the bug
We have several reports of our application not being able to shutdown. Apparently, some VaadinSessions stay alive and cannot be destroyed. Here the stack dumps of two such cases:
StackTraces1.txt
StackTraces2.txt
I found some common patterns in these dumps:
One of the threads blocks on a Jetty semaphore while reading the request content of an UIDL request. Note that this thread holds the lock on the VaadinSession while blocking.
A second thread blocks on the VaadinSession while trying to close the websocket:
A third thread also blocks on the VaadinSession while handling a connection loss, but this one comes from the HeartbeatInterception:
I'm not sure where things go wrong, but this does look a bit suspicious to me.
Expected behavior
The application should shut down without getting stuck in a deadlock.
Minimal reproducible example
Unfortunately, I don't have a reproducer. I hope that the stack traces are good enough.
Versions
The text was updated successfully, but these errors were encountered: