Skip to content

Commit

Permalink
Relax a DCHECK in the cursor serialization implementation to account …
Browse files Browse the repository at this point in the history
…for a streaming bug

Summary:
Due to a bug somewhere with streaming, the packet sent by the server ends up with an extra 0x00 byte at the end when received by the client, causing the DCHECK to fail.

Relaxing the DCHECK for now to unblock TAO/Sql over thrift usage of cursors. This relaxation of the check was discussed and agreed on offline, while we separately investigate and fix the extra byte bug.

Reviewed By: robertroeser

Differential Revision: D62780896

fbshipit-source-id: 456c9ca6e54933422bae4b2382e40314aba4597b
  • Loading branch information
Fadi Hanna authored and facebook-github-bot committed Sep 17, 2024
1 parent 0b059b7 commit 3386126
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion thrift/lib/cpp2/protocol/CursorBasedSerializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1108,8 +1108,11 @@ class CursorSerializationAdapter {
Protocol protForDebug;
protForDebug.setInput(buf.get());
protForDebug.skip(protocol::T_STRUCT);
// The check below should be using protForDebug.getCursorPosition() + 1,
// but due to a bug somewhere with streaming, an extra byte gets added
// to the buffer, causing this check to fail.
DCHECK_LE(
buf->computeChainDataLength(), protForDebug.getCursorPosition() + 1)
buf->computeChainDataLength(), protForDebug.getCursorPosition() + 2)
<< "Cursor serialization only supports messages containing a single struct or union.";
}

Expand Down

0 comments on commit 3386126

Please sign in to comment.