diff --git a/src/modules/udp_transport/source/udp_transport_impl.cc b/src/modules/udp_transport/source/udp_transport_impl.cc index e54edd0ef7..6ca61f5950 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.cc +++ b/src/modules/udp_transport/source/udp_transport_impl.cc @@ -80,7 +80,9 @@ class SocketFactory : public UdpTransportImpl::SocketFactoryInterface { } }; - +// Creates an UdpTransport using the definition of SocketFactory above, +// and passes (creating if needed) a pointer to the static singleton +// UdpSocketManager. UdpTransport* UdpTransport::Create(const WebRtc_Word32 id, WebRtc_UWord8& numSocketThreads) { @@ -89,11 +91,15 @@ UdpTransport* UdpTransport::Create(const WebRtc_Word32 id, UdpSocketManager::Create(id, numSocketThreads)); } +// Deletes the UdpTransport and decrements the refcount of the +// static singleton UdpSocketManager, possibly destroying it. +// Should only be used on UdpTransports that are created using Create. void UdpTransport::Destroy(UdpTransport* module) { if(module) { delete module; + UdpSocketManager::Return(); } } @@ -170,7 +176,6 @@ UdpTransportImpl::~UdpTransportImpl() delete _critPacketCallback; delete _cachLock; delete _socket_creator; - UdpSocketManager::Return(); WEBRTC_TRACE(kTraceMemory, kTraceTransport, _id, "%s deleted", __FUNCTION__); diff --git a/src/modules/udp_transport/source/udp_transport_impl.h b/src/modules/udp_transport/source/udp_transport_impl.h index 2c9530d101..9f4fd9fb06 100644 --- a/src/modules/udp_transport/source/udp_transport_impl.h +++ b/src/modules/udp_transport/source/udp_transport_impl.h @@ -35,8 +35,8 @@ public: }; // Constructor, only called by UdpTransport::Create and tests. - // The constructor takes ownership of the "maker", - // and will call Return on the socket_manager at exit. + // The constructor takes ownership of the "maker". + // The constructor does not take ownership of socket_manager. UdpTransportImpl(const WebRtc_Word32 id, SocketFactoryInterface* maker, UdpSocketManager* socket_manager); diff --git a/src/modules/udp_transport/source/udp_transport_unittest.cc b/src/modules/udp_transport/source/udp_transport_unittest.cc index 25a9b9d5f1..3125e2e5fb 100644 --- a/src/modules/udp_transport/source/udp_transport_unittest.cc +++ b/src/modules/udp_transport/source/udp_transport_unittest.cc @@ -45,6 +45,10 @@ class MockUdpSocketWrapper : public webrtc::UdpSocketWrapper { class MockUdpSocketManager : public webrtc::UdpSocketManager { public: + // Access to protected destructor. + void Destroy() { + delete this; + } MOCK_METHOD2(Init, bool(WebRtc_Word32, WebRtc_UWord8&)); MOCK_METHOD1(ChangeUniqueId, WebRtc_Word32(const WebRtc_Word32)); MOCK_METHOD0(Start, bool()); @@ -118,18 +122,20 @@ TEST_F(UDPTransportTest, ConstructorDoesNotCreateSocket) { webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, null_maker, null_manager); - webrtc::UdpTransport::Destroy(transport); + delete transport; } TEST_F(UDPTransportTest, InitializeSourcePorts) { WebRtc_Word32 id = 0; webrtc::UdpTransportImpl::SocketFactoryInterface* mock_maker = new MockSocketFactory(sockets_created()); - webrtc::UdpSocketManager* mock_manager = new MockUdpSocketManager(); + MockUdpSocketManager* mock_manager = new MockUdpSocketManager(); webrtc::UdpTransport* transport = new webrtc::UdpTransportImpl(id, mock_maker, mock_manager); EXPECT_EQ(0, transport->InitializeSourcePorts(4711, 4712)); EXPECT_EQ(2, NumSocketsCreated()); - webrtc::UdpTransport::Destroy(transport); + + delete transport; + mock_manager->Destroy(); }