Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-739

TCompactProtocol isn't suitable for reuse in partialDeserialize

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.3
    • 0.3
    • Java - Library
    • None

    Description

      Took me a bit to track this one down. Recently, I noticed that one of our applications that uses TDeserializer, TCompactProtocol, and the partialDeserialize method had a very large number of deserialization errors popping up. It turns out that when I implemented direct buffer access in a few spots, I also improved TDeserializer to reuse a single transport and protocol. However, once the partialDeserialize method gets what it's looking for, it returns without consuming the rest of the buffer, leaving a bunch of dangling field id deltas on its internal stack. This causes subsequent partialDeserialization attempts to fail. Once I got one failure, I'd get a new TDeserializer, so it looks like about 50% of my deserializations were failing.

      There seems to be two separate possible solutions for this problem. The first is to add a "reset" method to TProtocol that does nothing in most protocols, but stateful protocols implement so that they can clear themselves out. Then, TDeserializer calls this all the time. The other alternative is to add a "isReusable" or "isStateful" method to TProtocol, and just don't reuse those Protocols that are inappropriate. Personally, I prefer the former solution, but I'd love to hear others' thoughts.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            bryanduxbury Bryan Duxbury
            bryanduxbury Bryan Duxbury
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment