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

Deadlock when trying to enable data streaming #5117

Closed
daijithegeek opened this issue Aug 4, 2022 · 4 comments
Closed

Deadlock when trying to enable data streaming #5117

daijithegeek opened this issue Aug 4, 2022 · 4 comments

Comments

@daijithegeek
Copy link
Contributor

daijithegeek commented Aug 4, 2022

Hello,

I'm trying to use the Jersey streaming abilities by leveraging a StreamingOutput within a response.
The goal is to transfer a large file to a client leveraging the existing stack.

I spotted the two following issues while trying to do so:

  1. The JettyConnector readTimeout configuration is misleading. The PR Change JettyConnector 'readTimeout' behavior to match socket read tim… #5114 should fix the problem.
  2. When using async() to consume the streamed data, the Jetty connector buffers every byte in-memory before calling back the Jersey stack.

I need your opinion, especially on the second matter.
I tried to move the callback, and future completion right after the headers are received; that should work in theory, as all the necessary information is available at that point.
It worked well until I spotted an unexpected deadlock while using a regular endpoint (not streaming data) returning a JSON payload.

I provided an example of my use cases in the following PR: #5116
The test class contains both a streaming example and the deadlock example.

I believe that both use cases are valid. Can you help me to sort out this situation?
I was not able to find an escape hatch by myself, unfortunately.

Thanks a lot for your help!
David

@jansupol
Copy link
Contributor

Hi @daijithegeek
I tried your tests. Let's discuss testJettyThreadShouldNotDeadlock, first. For me, I was not able to run it, so I made a few small simplifications:
TEXT_PLAIN first:

        @GET
        @Produces(MediaType.TEXT_PLAIN)
        @Path("json")
        public String json() {
            return "great success";
        }

No AnObject second (no message body provider):

   public void testJettyThreadShouldNotDeadlock() throws Exception {
        String result = target("test")
//                .property(ClientProperties.READ_TIMEOUT, "10000") // remove this timeout to deadlock indefinitely
                .property(CommonProperties.OUTBOUND_CONTENT_LENGTH_BUFFER_SERVER, "-1")
                .path("json")
                .request()
                .async()
                .get(String.class)
                .get()
                ;

and it works fine no deadlock. Is it possible that your deadlock relates to the JSON?

@daijithegeek
Copy link
Contributor Author

daijithegeek commented Aug 11, 2022

Hi,
You're right. I forgot to change the resource implementation, so it transfers AnObject instance instead of a String.

Thank your for trying this. I'll try to clarify the problem I'm investigating.

Yes, the JSON deadlock is what I wanted to show. The problem arises when a MessageBodyReader awaits on the byte input stream - The JacksonJsonProvider being one example.

The conditions to be met are the following:

  • The data size exceeds the jetty data buffer - jetty will issue multiple org.eclipse.jetty.client.api.Response.Listener#onContent calls with partial data, using an internal thread
  • The MessageBodyReader awaits the input stream data - The javax.ws.rs.ext.MessageBodyReader#readFrom call blocks awaiting data on the Response entity.

The jetty thread is accountable for filling the entity stream is also the one that is blocked, awaiting the entity stream to be filled.

As an user the two choices are the following:
Current implementation

  • JacksonJsonProvider (and others) blocking MessageBodyReader` work fine
  • When using a StreamingOutput nothing is streamed; Bytes are buffered in memory until the whole data is available.

PR implementation

  • When using a StreamingOutput, the bytes arrive as soon as available (when jetty notifies through the onContentcallback).
  • JacksonJsonProvider (and others) blocking MessageBodyReader` are deadlocked.

I tried with ApacheConnector; both scenarios work properly

@jansupol
Copy link
Contributor

I wonder what is your message body reader. When I try your example, I get

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of org.glassfish.jersey.jetty.connector.StreamingTest$AnObject (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)

@daijithegeek
Copy link
Contributor Author

Thanks a lot for your patience...

I didn't realized the test involving JSON serialization was not ok with the current connector implementation.
Everything should be set properly now.

@jansupol jansupol closed this as completed Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants