[
https://issues.apache.org/jira/browse/THRIFT-867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13001463#comment-13001463
]
Nicholas Telford commented on THRIFT-867:
-----------------------------------------
On Arya's points on the best place to fix this:
* Arya's patch for the extension fixes a bug (namely that flush() gets called
when it shouldn't). Whether or not this causes things to break is besides the
point, it's unintended and should definitely be fixed.
* Having TFramedTransport write empty frames on flush, whether you're using
the extension or some other exotic protocol/transport, breaks servers that
adhere to the FramedTransport format. While I've not seen a specification for
this format, it seems implied that empty frames are not permitted. If this is
the case, we should definitely have TFramedTransport::flush() check the frame
isn't empty before writing it.
My argument for the second point is one of coupling: a method should never rely
on it's caller to call it in a particular order or validate it's state.
> PHP accelerator module's output transport is incompatible with
> TFramedTransport
> -------------------------------------------------------------------------------
>
> Key: THRIFT-867
> URL: https://issues.apache.org/jira/browse/THRIFT-867
> Project: Thrift
> Issue Type: Bug
> Components: PHP - Library
> Affects Versions: 0.4
> Reporter: Bryan Duxbury
> Fix For: 0.7
>
> Attachments: thrift-867.diff
>
>
> I think we've figured out the cause of everyone's problems with THRIFT-837.
> The patch itself is fine; however, in fixing that bug, we've exposed the fact
> that PHPOutputTransport erroneously calls down to the underlying PHP
> transport's flush() method every time the internal 8k buffer is flushed. This
> is fine for the buffered transport, but unacceptable for the framed
> transport, which should only be flushed once per RPC call.
> It seems like what we need to do is separate the "internal" buffer flushes
> from the "external" transport flushes. If we do that, everything should work
> out fine.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
