Jetty
  1. Jetty
  2. JETTY-309

Chunked encoding doesn't always flush the terminating 0

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 6.1.2rc2
    • Fix Version/s: 6.1.4rc0
    • Component/s: NIO
    • Labels:
      None
    • Number of attachments :
      3

      Description

      Looks like JETTY-274 isn't fixed yet: with the SVN changes as of revision 1770, I have another reproducible case, this time with a better packet capture, hopefully.

      In the attached capture, notice the three minute delay between packets #282 and #283, and that the only content in packet #283 is the terminating chunk length. The three minute delay is due to the client timeout: it closes the connection after three minutes, at which point jetty flushes the buffer. The data sat in the server the whole time.

      1. chunked.cap
        106 kB
        Anders Wallgren
      2. chunked.pkt
        107 kB
        Anders Wallgren
      3. jetty-309.patch
        2 kB
        Greg Wilkins

        Activity

        Hide
        Anders Wallgren added a comment -

        Same capture, different format, just in case.

        Show
        Anders Wallgren added a comment - Same capture, different format, just in case.
        Hide
        Greg Wilkins added a comment -

        Anders,

        If you could get a capture of traffic two ways that would be helpful so I can see what provoked the eventual flush.
        Also if you could describe the servlet/handler that is generating the content... ie reader or stream, lots of write or
        few big writes etc etc.

        thanks

        Show
        Greg Wilkins added a comment - Anders, If you could get a capture of traffic two ways that would be helpful so I can see what provoked the eventual flush. Also if you could describe the servlet/handler that is generating the content... ie reader or stream, lots of write or few big writes etc etc. thanks
        Hide
        Greg Wilkins added a comment -

        doh! your trace is 2 way!?!? really confused now

        Show
        Greg Wilkins added a comment - doh! your trace is 2 way!?!? really confused now
        Hide
        Anders Wallgren added a comment -

        The handler is using a writer and it's one single write.

        Show
        Anders Wallgren added a comment - The handler is using a writer and it's one single write.
        Hide
        Greg Wilkins added a comment -

        I think there are two issues here.

        One - the response should not be chunked anyway! The client asked for Connection: close
        so there is no need for chunking.

        Two- why the end chunk is not flushed.

        I haved fixe one (in svn), but still cannot reproduce 2 even with a single write of exactly the same size???

        Is reproducable for you (ie everytime) or only occasionally? under load or mostly idle?

        Show
        Greg Wilkins added a comment - I think there are two issues here. One - the response should not be chunked anyway! The client asked for Connection: close so there is no need for chunking. Two- why the end chunk is not flushed. I haved fixe one (in svn), but still cannot reproduce 2 even with a single write of exactly the same size??? Is reproducable for you (ie everytime) or only occasionally? under load or mostly idle?
        Hide
        Anders Wallgren added a comment -

        I just ran the test again a few times (same source as before): in five test runs on a dual core machine, it did not reproduce. In three test runs on an eight-core machine, it happens every time.

        The test does place a reasonably high load on our code, but not in the area where we use jetty. The client that drives the test is single-threaded and the only client that talks to the XML api, which is where jetty is used. In other words, there might be lots of context switching happening in other parts of the code even if jetty itself is not stressed.

        Show
        Anders Wallgren added a comment - I just ran the test again a few times (same source as before): in five test runs on a dual core machine, it did not reproduce. In three test runs on an eight-core machine, it happens every time. The test does place a reasonably high load on our code, but not in the area where we use jetty. The client that drives the test is single-threaded and the only client that talks to the XML api, which is where jetty is used. In other words, there might be lots of context switching happening in other parts of the code even if jetty itself is not stressed.
        Hide
        Greg Wilkins added a comment -

        This is with the latest svn head (r1774)? I hope not as it should not do a chunked response for this request of yours as it is connection: close.

        any way... the 8 cpu things definitely makes it sound like I have a race where jetty thinks a connection is complete when it is not.

        Head should fix your test, but if you remove connection:close, you will probably get the issue again.

        Show
        Greg Wilkins added a comment - This is with the latest svn head (r1774)? I hope not as it should not do a chunked response for this request of yours as it is connection: close. any way... the 8 cpu things definitely makes it sound like I have a race where jetty thinks a connection is complete when it is not. Head should fix your test, but if you remove connection:close, you will probably get the issue again.
        Hide
        Greg Wilkins added a comment -

        This patch is a speculative fix for this issue.

        Show
        Greg Wilkins added a comment - This patch is a speculative fix for this issue.
        Hide
        Greg Wilkins added a comment -

        Anders, can you try this patch (or mail me an 8 cpu box

        Show
        Greg Wilkins added a comment - Anders, can you try this patch (or mail me an 8 cpu box
        Hide
        Anders Wallgren added a comment -

        The patch seems to have resolved the problem. Thanks!

        Show
        Anders Wallgren added a comment - The patch seems to have resolved the problem. Thanks!
        Hide
        Greg Wilkins added a comment -

        Thanks. I have committed this patch.

        I think it is an overzealous solution and after 6.1.2 I may try to optimize a little.
        but better safe than sorry for now.

        Show
        Greg Wilkins added a comment - Thanks. I have committed this patch. I think it is an overzealous solution and after 6.1.2 I may try to optimize a little. but better safe than sorry for now.
        Hide
        jhahm added a comment -

        Hi, I'm confused by this comment from Greg: "Head should fix your test, but if you remove connection:close, you will probably get the issue again."

        Yet the bug is marked resolved. Is it fixed only for non-persistent connection case?

        Show
        jhahm added a comment - Hi, I'm confused by this comment from Greg: "Head should fix your test, but if you remove connection:close, you will probably get the issue again." Yet the bug is marked resolved. Is it fixed only for non-persistent connection case?
        Hide
        Greg Wilkins added a comment -

        jhahm,

        There were too issues: 1) bad handling of a Connection:close and 2) non flushing.
        I fixed them in two stages and the comment you refer to was after the first fix.
        Where I had fixed the handling of Connection:close so the response in Anders test
        would no longer be chunked. but if he modified his client to not request closed
        connections, the response would be chunked again and the flush problem would be
        back.

        But now both issues are fixed and thus the issue is resolved.... unless you can report
        differently.

        cheers

        Show
        Greg Wilkins added a comment - jhahm, There were too issues: 1) bad handling of a Connection:close and 2) non flushing. I fixed them in two stages and the comment you refer to was after the first fix. Where I had fixed the handling of Connection:close so the response in Anders test would no longer be chunked. but if he modified his client to not request closed connections, the response would be chunked again and the flush problem would be back. But now both issues are fixed and thus the issue is resolved.... unless you can report differently. cheers
        Hide
        Greg Wilkins added a comment -

        Nothing like actually doing the release to trigger new bug reports!
        It looks like this was only partially fixed in 6.1.2 as another report has been received of unflushed and unclosed responses.

        Show
        Greg Wilkins added a comment - Nothing like actually doing the release to trigger new bug reports! It looks like this was only partially fixed in 6.1.2 as another report has been received of unflushed and unclosed responses.
        Hide
        Greg Wilkins added a comment -

        I believe I have now really fixed this one.
        The previous fixed made sure that !writable status was set if connection.handle existed when the response was not complete.
        This was to ensure the connection would be dispatched again.

        However, another bug meant that sometimes the !writeable state was cleared but the connection was not dispatched.

        So in SVN head now is a fix that only clears the status once the connection is dispatched AFTER a select and not
        when the interested ops are set.

        I test harness has been added that simulates a write returning 0 or 1 bytes as being written. This duplicated the
        problem and the fix works at least for the test harness.

        If we can confirm this fix, I will look to do a rapid 6.1.3 with just this change. I will make a 6.1.3rc0 tag to assist
        testing this.

        Show
        Greg Wilkins added a comment - I believe I have now really fixed this one. The previous fixed made sure that !writable status was set if connection.handle existed when the response was not complete. This was to ensure the connection would be dispatched again. However, another bug meant that sometimes the !writeable state was cleared but the connection was not dispatched. So in SVN head now is a fix that only clears the status once the connection is dispatched AFTER a select and not when the interested ops are set. I test harness has been added that simulates a write returning 0 or 1 bytes as being written. This duplicated the problem and the fix works at least for the test harness. If we can confirm this fix, I will look to do a rapid 6.1.3 with just this change. I will make a 6.1.3rc0 tag to assist testing this.
        Hide
        Greg Wilkins added a comment -
        Show
        Greg Wilkins added a comment - http://svn.codehaus.org/jetty/jetty/tags/jetty-6.1.3rc0 is now available to checkout
        Hide
        Greg Wilkins added a comment -

        I believe this was fixed in 6.1.3, but some more buffering/flushing issues have been fixed in trunk, so please test 6.1.4rc0

        Show
        Greg Wilkins added a comment - I believe this was fixed in 6.1.3, but some more buffering/flushing issues have been fixed in trunk, so please test 6.1.4rc0

          People

          • Assignee:
            Greg Wilkins
            Reporter:
            Anders Wallgren
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: