Merge pull request #13 from paullouisageneau/stop-method

Stop transports before destruction

CMakeLists.txt

@@ -8,26 +8,23 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/Modules)
 
 set(LIBDATACHANNEL_SOURCES
-	candidate.cpp
-	certificate.cpp
-	channel.cpp
-	configuration.cpp
-	datachannel.cpp
-	description.cpp
-	dtlstransport.cpp
-	icetransport.cpp
-	peerconnection.cpp
-	rtc.cpp
-	sctptransport.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/candidate.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/certificate.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/channel.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/configuration.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/datachannel.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/description.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/dtlstransport.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/icetransport.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/peerconnection.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/rtc.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/sctptransport.cpp
 )
 
 set(TESTS_SOURCES
-    main.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/test/main.cpp
 )
 
-list(TRANSFORM LIBDATACHANNEL_SOURCES PREPEND ${CMAKE_CURRENT_SOURCE_DIR}/src/)
-list(TRANSFORM TESTS_SOURCES PREPEND ${CMAKE_CURRENT_SOURCE_DIR}/test/)
-
 # Hack because usrsctp uses CMAKE_SOURCE_DIR instead of CMAKE_CURRENT_SOURCE_DIR
 set(CMAKE_REQUIRED_FLAGS "-I${CMAKE_CURRENT_SOURCE_DIR}/usrsctp/usrsctplib")
 
@@ -40,8 +37,17 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   # using regular Clang or AppleClang: Needed since they don't have -Wno-error=format-truncation
   target_compile_options(usrsctp-static PRIVATE -Wno-error=address-of-packed-member)
 else()
-  # all other compilers
-  target_compile_options(usrsctp-static PRIVATE -Wno-error=address-of-packed-member -Wno-error=format-truncation)
+  if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+    if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "9.0")
+      # GCC version below 9.0 does not support option address-of-packed-member
+	  target_compile_options(usrsctp-static PRIVATE -Wno-error=format-truncation)
+	else()
+	  target_compile_options(usrsctp-static PRIVATE -Wno-error=address-of-packed-member -Wno-error=format-truncation)
+	endif()  
+  else()
+    # all other compilers
+    target_compile_options(usrsctp-static PRIVATE -Wno-error=address-of-packed-member -Wno-error=format-truncation)
+  endif()
 endif()
 
 option(USE_GNUTLS "Use GnuTLS instead of OpenSSL" OFF)

src/dtlstransport.cpp

@@ -85,17 +85,19 @@ DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, shared_ptr<Certific
 }
 
 DtlsTransport::~DtlsTransport() {
-	resetLower();
-
-	mIncomingQueue.stop();
-	mRecvThread.join();
-
 	gnutls_bye(mSession, GNUTLS_SHUT_RDWR);
 	gnutls_deinit(mSession);
 }
 
 DtlsTransport::State DtlsTransport::state() const { return mState; }
 
+void DtlsTransport::stop() {
+	Transport::stop();
+
+	mIncomingQueue.stop();
+	mRecvThread.join();
+}
+
 bool DtlsTransport::send(message_ptr message) {
 	if (!message || mState != State::Connected)
 		return false;
@@ -356,16 +358,18 @@ DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, shared_ptr<Certific
 }
 
 DtlsTransport::~DtlsTransport() {
-	resetLower();
-
-	mIncomingQueue.stop();
-	mRecvThread.join();
-
 	SSL_shutdown(mSsl);
 	SSL_free(mSsl);
 	SSL_CTX_free(mCtx);
 }
 
+void DtlsTransport::stop() {
+	Transport::stop();
+
+	mIncomingQueue.stop();
+	mRecvThread.join();
+}
+
 DtlsTransport::State DtlsTransport::state() const { return mState; }
 
 bool DtlsTransport::send(message_ptr message) {

src/dtlstransport.hpp

@@ -54,6 +54,7 @@ public:
 
 	State state() const;
 
+	void stop() override;
 	bool send(message_ptr message); // false if dropped
 
 private:

src/icetransport.cpp

@@ -130,7 +130,9 @@ IceTransport::IceTransport(const Configuration &config, Description::Role role,
 	                       RecvCallback, this);
 }
 
-IceTransport::~IceTransport() {
+IceTransport::~IceTransport() {}
+
+void IceTransport::stop() {
 	g_main_loop_quit(mMainLoop.get());
 	mMainLoopThread.join();
 }

src/icetransport.hpp

@@ -67,7 +67,8 @@ public:
 	std::optional<string> getLocalAddress() const;
 	std::optional<string> getRemoteAddress() const;
 
-	bool send(message_ptr message); // false if dropped
+	void stop() override;
+	bool send(message_ptr message) override; // false if dropped
 
 private:
 	void incoming(message_ptr message);

src/peerconnection.cpp

@@ -37,6 +37,13 @@ PeerConnection::PeerConnection(const Configuration &config)
     : mConfig(config), mCertificate(make_certificate("libdatachannel")), mState(State::New) {}
 
 PeerConnection::~PeerConnection() {
+	if (mIceTransport)
+		mIceTransport->stop();
+	if (mDtlsTransport)
+		mDtlsTransport->stop();
+	if (mSctpTransport)
+		mSctpTransport->stop();
+
 	mSctpTransport.reset();
 	mDtlsTransport.reset();
 	mIceTransport.reset();

src/sctptransport.cpp

@@ -143,8 +143,19 @@ SctpTransport::SctpTransport(std::shared_ptr<Transport> lower, uint16_t port,
 }
 
 SctpTransport::~SctpTransport() {
-	resetLower();
-	onRecv(nullptr); // unset recv callback
+	if (mSock) {
+		usrsctp_shutdown(mSock, SHUT_RDWR);
+		usrsctp_close(mSock);
+	}
+
+	usrsctp_deregister_address(this);
+	GlobalCleanup();
+}
+
+SctpTransport::State SctpTransport::state() const { return mState; }
+
+void SctpTransport::stop() {
+	Transport::stop();
 
 	mSendQueue.stop();
 
@@ -154,14 +165,6 @@ SctpTransport::~SctpTransport() {
 		mConnectDataSent = true;
 		mConnectCondition.notify_all();
 	}
-
-	if (mSock) {
-		usrsctp_shutdown(mSock, SHUT_RDWR);
-		usrsctp_close(mSock);
-	}
-
-	usrsctp_deregister_address(this);
-	GlobalCleanup();
 }
 
 void SctpTransport::connect() {
@@ -186,8 +189,6 @@ void SctpTransport::connect() {
 		throw std::runtime_error("Connection attempt failed, errno=" + std::to_string(errno));
 }
 
-SctpTransport::State SctpTransport::state() const { return mState; }
-
 bool SctpTransport::send(message_ptr message) {
 	std::lock_guard<std::mutex> lock(mSendMutex);
 

src/sctptransport.hpp

@@ -50,7 +50,8 @@ public:
 
 	State state() const;
 
-	bool send(message_ptr message); // false if buffered
+	void stop() override;
+	bool send(message_ptr message) override; // false if buffered
 	void reset(unsigned int stream);
 
 private:

src/transport.hpp

@@ -36,13 +36,16 @@ public:
 		if (auto lower = std::atomic_load(&mLower))
 			lower->onRecv(std::bind(&Transport::incoming, this, _1));
 	}
-	virtual ~Transport() { resetLower(); }
+	virtual ~Transport() {}
 
+	virtual void stop() { resetLower(); }
 	virtual bool send(message_ptr message) = 0;
+
 	void onRecv(message_callback callback) { mRecvCallback = std::move(callback); }
 
 protected:
 	void recv(message_ptr message) { mRecvCallback(message); }
+
 	void resetLower() {
 		if (auto lower = std::atomic_exchange(&mLower, std::shared_ptr<Transport>(nullptr)))
 			lower->onRecv(nullptr);