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
|
auto controller = getController(); |
, controller is created as shared_ptr, but used as raw pointer in
. 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: ad9fd63
My suggestion is to revert the above commit.
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
proxygen/proxygen/lib/http/session/HTTPSessionAcceptor.cpp
Line 71 in 3f8d21f
proxygen/proxygen/lib/http/session/HTTPSessionAcceptor.cpp
Line 99 in 3f8d21f
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: ad9fd63
My suggestion is to revert the above commit.