commit cd54cbb82ab834524334813fdc275aae46142d11
parent 99392821594702a12cb6381b5dbb438de49593f1
Author: Dan Baker <dbaker@mozilla.com>
Date: Mon, 1 Dec 2025 18:52:03 -0700
Bug 2000941 - Vendor libwebrtc from fd381db407
Upstream commit: https://webrtc.googlesource.com/src/+/fd381db407aeb2a72e00083fabddcdb02389ea97
Clarify Socket ownership in TurnServer using type system
Change socket maps that owns keys to use unique_ptr for keys.
This automates deallocation of the sockets and requires to be more careful when accessing the map.
Bug: None
Change-Id: I9f7f810a5b9bcaf3974a80f01b140cd029878a2b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/408821
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45612}
Diffstat:
3 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/third_party/libwebrtc/README.mozilla.last-vendor b/third_party/libwebrtc/README.mozilla.last-vendor
@@ -1,4 +1,4 @@
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /Users/danielbaker/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
-libwebrtc updated from /Users/danielbaker/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2025-12-02T01:48:55.251832+00:00.
+libwebrtc updated from /Users/danielbaker/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2025-12-02T01:51:44.556439+00:00.
# base of lastest vendoring
-ef084b6cc6
+fd381db407
diff --git a/third_party/libwebrtc/p2p/test/turn_server.cc b/third_party/libwebrtc/p2p/test/turn_server.cc
@@ -68,6 +68,17 @@ bool IsTurnChannelData(uint16_t msg_type) {
return ((msg_type & 0xC000) == 0x4000);
}
+// Helper to search a map keyed by unique_ptr using raw pointer.
+template <typename Map, typename Key>
+Map::iterator FindByRawPtr(Map& map, Key* ptr) {
+ // Wrap raw pointer to unique_ptr to be able to compare it with map keys.
+ std::unique_ptr<Key> uptr = absl::WrapUnique(ptr);
+ typename Map::iterator iter = map.find(uptr);
+ // Do not forget to release ownership as `ptr` is passed without ownership.
+ uptr.release();
+ return iter;
+}
+
} // namespace
int GetStunSuccessResponseTypeOrZero(const StunMessage& req) {
@@ -97,19 +108,12 @@ TurnServer::TurnServer(const Environment& env, TaskQueueBase* thread)
TurnServer::~TurnServer() {
RTC_DCHECK_RUN_ON(thread_);
- for (auto& [socket, unused] : server_sockets_) {
- delete socket;
- }
-
- for (auto& [socket, unused] : server_listen_sockets_) {
- delete socket;
- }
}
void TurnServer::AddInternalSocket(std::unique_ptr<AsyncPacketSocket> socket,
ProtocolType protocol) {
RTC_DCHECK_RUN_ON(thread_);
- auto [it, inserted] = server_sockets_.emplace(socket.release(), protocol);
+ auto [it, inserted] = server_sockets_.emplace(std::move(socket), protocol);
RTC_DCHECK(inserted);
it->first->RegisterReceivedPacketCallback(
[&](AsyncPacketSocket* socket, const ReceivedIpPacket& packet) {
@@ -124,7 +128,7 @@ void TurnServer::AddInternalServerSocket(
std::unique_ptr<SSLAdapterFactory> ssl_adapter_factory) {
RTC_DCHECK_RUN_ON(thread_);
auto [iter, inserted] = server_listen_sockets_.emplace(
- socket.release(),
+ std::move(socket),
ServerSocketInfo{.proto = protocol,
.ssl_adapter_factory = std::move(ssl_adapter_factory)});
RTC_DCHECK(inserted);
@@ -141,20 +145,15 @@ void TurnServer::SetExternalSocketFactory(PacketSocketFactory* factory,
void TurnServer::OnNewInternalConnection(Socket* socket) {
RTC_DCHECK_RUN_ON(thread_);
- RTC_DCHECK(server_listen_sockets_.find(socket) !=
- server_listen_sockets_.end());
- AcceptConnection(socket);
-}
-
-void TurnServer::AcceptConnection(Socket* server_socket) {
- RTC_DCHECK_RUN_ON(thread_);
+ auto iter = FindByRawPtr(server_listen_sockets_, socket);
+ RTC_DCHECK(iter != server_listen_sockets_.end());
// Check if someone is trying to connect to us.
SocketAddress accept_addr;
std::unique_ptr<Socket> accepted_socket =
- absl::WrapUnique(server_socket->Accept(&accept_addr));
+ absl::WrapUnique(socket->Accept(&accept_addr));
if (accepted_socket != nullptr) {
- const ServerSocketInfo& info = server_listen_sockets_[server_socket];
+ const ServerSocketInfo& info = iter->second;
if (info.ssl_adapter_factory) {
std::unique_ptr<SSLAdapter> ssl_adapter = absl::WrapUnique(
info.ssl_adapter_factory->CreateAdapter(accepted_socket.release()));
@@ -175,7 +174,10 @@ void TurnServer::AcceptConnection(Socket* server_socket) {
void TurnServer::OnInternalSocketClose(AsyncPacketSocket* socket, int err) {
RTC_DCHECK_RUN_ON(thread_);
- DestroyInternalSocket(socket);
+ if (auto iter = FindByRawPtr(server_sockets_, socket);
+ iter != server_sockets_.end()) {
+ DestroyInternalSocket(iter);
+ }
}
void TurnServer::OnInternalPacket(AsyncPacketSocket* socket,
@@ -185,7 +187,7 @@ void TurnServer::OnInternalPacket(AsyncPacketSocket* socket,
if (packet.payload().size() < TURN_CHANNEL_HEADER_SIZE) {
return;
}
- auto iter = server_sockets_.find(socket);
+ auto iter = FindByRawPtr(server_sockets_, socket);
RTC_DCHECK(iter != server_sockets_.end());
TurnServerConnection conn(packet.source_address(), iter->second, socket);
uint16_t msg_type = GetBE16(packet.payload().data());
@@ -508,32 +510,28 @@ void TurnServer::Send(TurnServerConnection* conn, const ByteBufferWriter& buf) {
void TurnServer::DestroyAllocation(TurnServerAllocation* allocation) {
// Removing the internal socket if the connection is not udp.
AsyncPacketSocket* socket = allocation->conn()->socket();
- auto iter = server_sockets_.find(socket);
+ auto iter = FindByRawPtr(server_sockets_, socket);
// Skip if the socket serving this allocation is UDP, as this will be shared
// by all allocations.
// Note: We may not find a socket if it's a TCP socket that was closed, and
// the allocation is only now timing out.
if (iter != server_sockets_.end() && iter->second != PROTO_UDP) {
- DestroyInternalSocket(socket);
+ DestroyInternalSocket(iter);
}
allocations_.erase(*(allocation->conn()));
}
-void TurnServer::DestroyInternalSocket(AsyncPacketSocket* socket) {
- auto iter = server_sockets_.find(socket);
- if (iter != server_sockets_.end()) {
- AsyncPacketSocket* server_socket = iter->first;
- server_socket->UnsubscribeCloseEvent(this);
- server_socket->DeregisterReceivedPacketCallback();
- server_sockets_.erase(iter);
- std::unique_ptr<AsyncPacketSocket> socket_to_delete =
- absl::WrapUnique(server_socket);
- // We must destroy the socket async to avoid invalidating the sigslot
- // callback list iterator inside a sigslot callback. (In other words,
- // deleting an object from within a callback from that object).
- thread_->PostTask([socket_to_delete = std::move(socket_to_delete)] {});
- }
+void TurnServer::DestroyInternalSocket(ServerSocketMap::iterator iter) {
+ RTC_DCHECK(iter != server_sockets_.end());
+ auto node = server_sockets_.extract(iter);
+ AsyncPacketSocket* server_socket = node.key().get();
+ server_socket->UnsubscribeCloseEvent(this);
+ server_socket->DeregisterReceivedPacketCallback();
+ // We must destroy the socket async to avoid invalidating the callback list
+ // iterator. (In other words, deleting an object from within a callback from
+ // that object).
+ thread_->PostTask([node = std::move(node)] {});
}
TurnServerConnection::TurnServerConnection(const SocketAddress& src,
diff --git a/third_party/libwebrtc/p2p/test/turn_server.h b/third_party/libwebrtc/p2p/test/turn_server.h
@@ -270,6 +270,9 @@ class TurnServer : public sigslot::has_slots<> {
}
private:
+ using ServerSocketMap =
+ std::map<std::unique_ptr<AsyncPacketSocket>, ProtocolType>;
+
// All private member functions and variables should have access restricted to
// thread_. But compile-time annotations are missing for members access from
// TurnServerAllocation (via friend declaration).
@@ -279,9 +282,6 @@ class TurnServer : public sigslot::has_slots<> {
const ReceivedIpPacket& packet) RTC_RUN_ON(thread_);
void OnNewInternalConnection(Socket* socket);
-
- // Accept connections on this server socket.
- void AcceptConnection(Socket* server_socket) RTC_RUN_ON(thread_);
void OnInternalSocketClose(AsyncPacketSocket* socket, int err);
void HandleStunMessage(TurnServerConnection* conn,
@@ -325,7 +325,8 @@ class TurnServer : public sigslot::has_slots<> {
void Send(TurnServerConnection* conn, const ByteBufferWriter& buf);
void DestroyAllocation(TurnServerAllocation* allocation) RTC_RUN_ON(thread_);
- void DestroyInternalSocket(AsyncPacketSocket* socket) RTC_RUN_ON(thread_);
+ void DestroyInternalSocket(ServerSocketMap::iterator iter)
+ RTC_RUN_ON(thread_);
struct ServerSocketInfo {
ProtocolType proto;
@@ -348,12 +349,8 @@ class TurnServer : public sigslot::has_slots<> {
// Check for permission when receiving an external packet.
bool enable_permission_checks_ = true;
- // `server_sockets_` and `server_listen_sockets_` use raw pointers as keys,
- // but de-facto they should be unique_ptr. It is responsibility of the
- // TurnServer to de-allocate keys when map entries are erased.
- std::map<AsyncPacketSocket*, ProtocolType> server_sockets_
- RTC_GUARDED_BY(thread_);
- std::map<Socket*, ServerSocketInfo> server_listen_sockets_
+ ServerSocketMap server_sockets_ RTC_GUARDED_BY(thread_);
+ std::map<std::unique_ptr<Socket>, ServerSocketInfo> server_listen_sockets_
RTC_GUARDED_BY(thread_);
std::unique_ptr<PacketSocketFactory> external_socket_factory_
RTC_GUARDED_BY(thread_);
