HTTPSessionAcceptor::getController() should return a raw pointer, instead of std::shared_ptr
- 主要言語
- C++
- スター
- 8.4k
- フォーク
- 1.5k
- 平均マージ
- 10分
- マージ済み PR(30日)
- 2
説明
The reason why it should return a raw pointer is because: One HTTPSessionController is mapped to one HTTPSession. An HTTPSessionAcceptor manages multiple controllers. It is better to follow the same lifetime management as HQSessionController, which return a raw pointer and let itself do self-destroyed in HQSessionController::detachSession().
Otherwise, in https://github.com/facebook/proxygen/blob/3f8d21fc2fb4def298785b28d79168e301438c20/proxygen/lib/http/session/HTTPSessionAcceptor.cpp#L71, controller is created as shared_ptr, but used as raw pointer in https://github.com/facebook/proxygen/blob/3f8d21fc2fb4def298785b28d79168e301438c20/proxygen/lib/http/session/HTTPSessionAcceptor.cpp#L99. Then it is auto-released after HTTPSessionAcceptor::onNewConnection is done.
It will increase a lot of burden on HTTPSessionAcceptor side to maintain a copy of std::shared_ptr to handle each HTTPSessionController's lifetime. Otherwise, without a copy inside HTTPSessionAcceptor, after HTTPSessionAcceptor::onNewConnection is done, HTTPSessionController will be released, then HTTPDownstreamSession will encounter a potential heap-use-after-free issue.
blame commit: https://github.com/facebook/proxygen/commit/ad9fd639d74f19fa771ebe04e64f1eb723073e37
My suggestion is to revert the above commit.
コントリビューションガイド
評価
この issue はまだ評価されていません。