Skip to content

Commit 91102db

Browse files
MikkoAtWorkjemoreira
authored andcommitted
Add state guard to WebRTC connection controller
WebRtcSessionDescriptionFactory forcibly flushes its callbacks during destruction. When the WebRtcSessionDescriptionFactory executes a pending callback, it runs after the parent PeerConnection has begun destruction. This results in an immediate crash if the callback attempts to access or modify members of the already dying PeerConnection object. Example backtrace: 0 webrtc::WebRtcSessionDescriptionFactory::SdesPolicy() const () 1 webrtc::SdpOfferAnswerHandler::ValidateSessionDescription() 2 webrtc::SdpOfferAnswerHandler::DoSetLocalDescription() 3 rtc::rtc_operations_chain_internal::OperationWithFunctor::Run() () 4 webrtc::SdpOfferAnswerHandler::SetLocalDescription() 5 webrtc::PeerConnectionProxyWithInternal() 6 webrtc_streaming::ConnectionController::OnCreateSDPSuccess() 7 CreateSessionDescriptionObserverIntermediate::OnSuccess() 8 WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() 9 webrtc::PeerConnection::~PeerConnection() () 10 rtc::RefCountedObject<webrtc::PeerConnection>::~RefCountedObject() () This change adds an explicit IsShuttingDown() state guard in the connection controller. This ensures that any callback triggered during destruction safely exits before attempting to access objects being destroyed.
1 parent 3585f61 commit 91102db

File tree

4 files changed

+21
-2
lines changed

4 files changed

+21
-2
lines changed

base/cvd/cuttlefish/host/frontend/webrtc/libcommon/connection_controller.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ ConnectionController::ConnectionController(
8989
ConnectionController::Observer& observer)
9090
: sig_handler_(sig_handler),
9191
connection_builder_(connection_builder),
92-
observer_(observer) {}
92+
observer_(observer),
93+
shutting_down_(false) {}
9394

9495
void ConnectionController::CreateOffer() {
9596
// No memory leak here because this is a ref counted object and the
@@ -114,6 +115,11 @@ Result<void> ConnectionController::RequestOffer(
114115
}
115116

116117
void ConnectionController::FailConnection(const std::string& message) {
118+
if (shutting_down_) {
119+
// Don't bother signaling the error during connection teardown
120+
LOG(ERROR) << message;
121+
return;
122+
}
117123
Json::Value reply;
118124
reply["type"] = "error";
119125
reply["error"] = message;
@@ -189,6 +195,7 @@ Result<void> ConnectionController::OnErrorMsg(const std::string& msg) {
189195

190196
Result<void> ConnectionController::OnCreateSDPSuccess(
191197
webrtc::SessionDescriptionInterface* desc) {
198+
CF_EXPECT(!shutting_down_, "Operation aborted during connection teardown.");
192199
std::string offer_str;
193200
desc->ToString(&offer_str);
194201
std::string sdp_type = desc->type();
@@ -225,6 +232,11 @@ void ConnectionController::OnSetLocalDescriptionFailure(
225232

226233
void ConnectionController::OnSetRemoteDescriptionComplete(
227234
const webrtc::RTCError& error) {
235+
if (shutting_down_) {
236+
LOG(WARNING) << "OnSetRemoteDescriptionComplete aborted on "
237+
<< "connection teardown";
238+
return;
239+
}
228240
if (!error.ok()) {
229241
// The remote description was rejected, can't connect to device.
230242
FailConnection(

base/cvd/cuttlefish/host/frontend/webrtc/libcommon/connection_controller.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ class ConnectionController : public webrtc::PeerConnectionObserver {
131131
void OnRemoveTrack(
132132
rtc::scoped_refptr<webrtc::RtpReceiverInterface> receiver) override;
133133

134+
void Shutdown() { shutting_down_ = true; }
135+
134136
private:
135137
friend class CreateSessionDescriptionObserverIntermediate;
136138
friend class SetSessionDescriptionObserverIntermediate;
@@ -166,6 +168,7 @@ class ConnectionController : public webrtc::PeerConnectionObserver {
166168
PeerConnectionBuilder& connection_builder_;
167169
Observer& observer_;
168170

171+
bool shutting_down_;
169172
rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection_;
170173
std::vector<std::unique_ptr<webrtc::IceCandidateInterface>>
171174
pending_ice_candidates_;

base/cvd/cuttlefish/host/frontend/webrtc/libdevice/client_handler.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ ClientHandler::ClientHandler(
8585
data_channels_handler_(observer),
8686
camera_track_(new ClientVideoTrackImpl()) {}
8787

88+
ClientHandler::~ClientHandler() {
89+
controller_.Shutdown();
90+
}
91+
8892
rtc::scoped_refptr<webrtc::RtpSenderInterface>
8993
ClientHandler::AddTrackToConnection(
9094
rtc::scoped_refptr<webrtc::MediaStreamTrackInterface> track,

base/cvd/cuttlefish/host/frontend/webrtc/libdevice/client_handler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class ClientHandler : public ConnectionController::Observer,
6060
PeerConnectionBuilder& connection_builder,
6161
std::function<void(const Json::Value&)> send_to_client_cb,
6262
std::function<void(bool)> on_connection_changed_cb);
63-
~ClientHandler() override = default;
63+
~ClientHandler() override;
6464

6565
bool AddDisplay(rtc::scoped_refptr<webrtc::VideoTrackInterface> track,
6666
const std::string& label);

0 commit comments

Comments
 (0)