facebook / facebook/proxygen

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

Offen
#554 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
C++
Sterne
8.4k
Forks
1.5k
Ø Merge
10 Min.
Gemergte PRs (30 T.)
2

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.