mirror of
https://github.com/facebook/proxygen.git
synced 2026-10-24 01:33:54 +03:00
Forward setParserPaused to the codec wrapped by a chain-end filter
Summary: Fix-forward for D123003021, root cause of S720522. D123003021 sends HTTPCodecFilterChain::setParserPaused straight to the chain end and makes HTTPCodecFilter::setParserPaused an empty final. That assumes the chain end is a concrete codec. It is not always: x2pagentd's forward-proxy port installs ConnectPassthroughCodec (a PassThroughHTTPCodecFilter that owns an inner HTTP1xCodec in call_) as the session codec through a codec factory, so the filter is the chain end. Since D123003021, every pause/resume from HTTPSession lands on the empty final and never reaches the inner HTTP1xCodec. A WebSocket upgrade through localhost:10054 then gets its 101, and the connection is torn down on the first client frame: the S720522 symptom on ~5k corp hosts. Make the final HTTPCodecFilter::setParserPaused forward to call_. The chain still calls the chain end directly, so filters in the middle of a chain are still skipped and the perf win of D123003021 stays; only a filter that is itself the chain end (or is called directly) forwards to the codec it wraps, as before D123003021. Reviewed By: hnovikov Differential Revision: D126459187 fbshipit-source-id: bef11d663d436c4d56484e55b4e5df54cae09e84
This commit is contained in:
committed by
meta-codesync[bot]
parent
146c196a8b
commit
3d4739b40c
@@ -24,8 +24,13 @@ class HTTPCodecFilter
|
||||
using GenericFilter::GenericFilter;
|
||||
|
||||
// Filters are not allowed to change this: HTTPCodecFilterChain sends it
|
||||
// straight to the codec at the end of the chain.
|
||||
void setParserPaused(bool /* paused */) final {
|
||||
// straight to the codec at the end of the chain and never calls this on
|
||||
// intermediate filters. Forward to call_ so that a filter used as the chain
|
||||
// destination reaches the codec it wraps.
|
||||
void setParserPaused(bool paused) final {
|
||||
if (call_) {
|
||||
call_->setParserPaused(paused);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -274,6 +274,30 @@ TEST_F(HTTPChecksTest, SetParserPausedReachesChainEnd) {
|
||||
chain_.setParserPaused(true);
|
||||
}
|
||||
|
||||
// A filter can itself be the chain's destination when it wraps its own codec
|
||||
// in call_; pausing the chain has to reach that inner codec.
|
||||
TEST(HTTPCodecFilterChainTest, SetParserPausedReachesCodecWrappedByChainEnd) {
|
||||
class WrappingCodec : public PassThroughHTTPCodecFilter {
|
||||
public:
|
||||
explicit WrappingCodec(HTTPCodec* inner) {
|
||||
call_ = inner;
|
||||
}
|
||||
void setCallback(HTTPCodec::Callback* callback) override {
|
||||
callback_ = callback;
|
||||
}
|
||||
};
|
||||
auto* inner = new MockHTTPCodec();
|
||||
MockHTTPCodecCallback callback;
|
||||
HTTPCodecFilterChain chain(std::make_unique<WrappingCodec>(inner));
|
||||
chain.setCallback(&callback);
|
||||
chain.add<HTTPChecks>();
|
||||
EXPECT_CALL(*inner, setParserPaused(true));
|
||||
EXPECT_CALL(*inner, setParserPaused(false));
|
||||
|
||||
chain.setParserPaused(true);
|
||||
chain.setParserPaused(false);
|
||||
}
|
||||
|
||||
TEST_F(DebugFilterTest, NoError) {
|
||||
chain_.onIngress(*makeIOBuf("foo"));
|
||||
chain_.onIngressEOF();
|
||||
|
||||
Reference in New Issue
Block a user