facebook / facebook/proxygen

HTTPSessionAcceptor::getController() should return a raw pointer, instead of std::shared_ptr

Aperta
#554 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
C++
Stelle
8.4k
Fork
1.5k
Merge medio
10m
PR unite (30g)
2

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.