make session_handle hold a weak_ptr and be copyable (#2034)

include/libtorrent/aux_/session_impl.hpp

@@ -231,6 +231,7 @@ namespace aux {
 			, boost::noncopyable
 			, single_threaded
 			, aux::error_handler_interface
+			, std::enable_shared_from_this<session_impl>
 		{
 			// the size of each allocation that is chained in the send buffer
 			enum { send_buffer_size_impl = 128 };

include/libtorrent/error_code.hpp

@@ -320,6 +320,8 @@ namespace libtorrent {
 			// peer was banned because its listen port is within a banned port
 			// range, as specified by the port_filter.
 			banned_by_port_filter,
+			// The session_handle is not referring to a valid session_impl
+			invalid_session_handle,
 
 
 			// The NAT-PMP router responded with an unsupported protocol version

include/libtorrent/session.hpp

@@ -184,7 +184,6 @@ namespace aux {
 		// configuring it, you can pass in a session_params object. Its settings
 		// will take effect before the session starts up.
 		explicit session(session_params params = session_params())
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			start(std::move(params), nullptr);
@@ -204,7 +203,6 @@ namespace aux {
 		// 	destruct the session object, then sync with the io_service, then
 		// 	destruct the session_proxy object.
 		session(session_params params, io_service& ios)
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			start(std::move(params), &ios);
@@ -223,7 +221,6 @@ namespace aux {
 		// pass 0 as the flags parameter.
 		session(settings_pack pack
 			, int flags = start_default_features | add_default_plugins)
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			start(flags, std::move(pack), nullptr);
@@ -253,7 +250,6 @@ namespace aux {
 		session(settings_pack pack
 			, io_service& ios
 			, int flags = start_default_features | add_default_plugins)
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			start(flags, std::move(pack), &ios);
@@ -276,7 +272,6 @@ namespace aux {
 		session(fingerprint const& print
 			, int flags = start_default_features | add_default_plugins
 			, std::uint32_t alert_mask = alert::error_notification)
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			settings_pack pack;
@@ -299,7 +294,6 @@ namespace aux {
 			, char const* listen_interface = "0.0.0.0"
 			, int flags = start_default_features | add_default_plugins
 			, int alert_mask = alert::error_notification)
-			: session_handle(nullptr)
 		{
 			TORRENT_CFG();
 			TORRENT_ASSERT(listen_port_range.first > 0);

include/libtorrent/session_handle.hpp

@@ -69,21 +69,17 @@ namespace libtorrent {
 
 	struct TORRENT_EXPORT session_handle
 	{
-		session_handle() : m_impl(nullptr) {}
+		friend class session;
+		friend struct aux::session_impl;
 
-		explicit session_handle(aux::session_impl* impl)
+		session_handle() {}
-			: m_impl(impl)
-		{}
 
-		// moveable
+		session_handle(session_handle const& t) = default;
-		session_handle(session_handle&&) = default;
+		session_handle(session_handle&& t) = default;
+		session_handle& operator=(session_handle const&) = default;
 		session_handle& operator=(session_handle&&) = default;
 
-		// non copyable
+		bool is_valid() const { return !m_impl.expired(); }
-		session_handle(session_handle const&) = delete;
-		session_handle& operator=(session_handle const&) = delete;
-
-		bool is_valid() const { return m_impl != nullptr; }
 
 		// TODO: 2 the ip filter should probably be saved here too
 
@@ -1021,8 +1017,8 @@ namespace libtorrent {
 
 		// This function is intended only for use by plugins. This type does
 		// not have a stable API and should be relied on as little as possible.
-		aux::session_impl* native_handle() const
+		std::shared_ptr<aux::session_impl> native_handle() const
-		{ return m_impl; }
+		{ return m_impl.lock(); }
 
 	private:
 
@@ -1035,7 +1031,11 @@ namespace libtorrent {
 		template <typename Ret, typename Fun, typename... Args>
 		Ret sync_call_ret(Fun f, Args&&... a) const;
 
-		aux::session_impl* m_impl;
+		explicit session_handle(std::weak_ptr<aux::session_impl> impl)
+			: m_impl(impl)
+		{}
+
+		std::weak_ptr<aux::session_impl> m_impl;
 	};
 
 } // namespace libtorrent

src/error_code.cpp

@@ -168,7 +168,7 @@ namespace libtorrent {
 			"invalid SSL certificate",
 			"not an SSL torrent",
 			"banned by port filter",
-			"",
+			"invalid session handle used",
 			"",
 			"",
 			"",

src/session.cpp

@@ -306,7 +306,7 @@ namespace libtorrent {
 		}
 
 		m_impl = std::make_shared<aux::session_impl>(*ios);
-		*static_cast<session_handle*>(this) = session_handle(m_impl.get());
+		*static_cast<session_handle*>(this) = session_handle(m_impl);
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
 		for (auto const& ext : params.extensions)

src/session_handle.cpp

@@ -54,19 +54,21 @@ namespace libtorrent {
 	template <typename Fun, typename... Args>
 	void session_handle::async_call(Fun f, Args&&... a) const
 	{
-		m_impl->get_io_service().dispatch([=]() mutable
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+		s->get_io_service().dispatch([=]() mutable
 		{
 #ifndef BOOST_NO_EXCEPTIONS
 			try {
 #endif
-				(m_impl->*f)(std::forward<Args>(a)...);
+				(s.get()->*f)(std::forward<Args>(a)...);
 #ifndef BOOST_NO_EXCEPTIONS
 			} catch (system_error const& e) {
-				m_impl->alerts().emplace_alert<session_error_alert>(e.code(), e.what());
+				s->alerts().emplace_alert<session_error_alert>(e.code(), e.what());
 			} catch (std::exception const& e) {
-				m_impl->alerts().emplace_alert<session_error_alert>(error_code(), e.what());
+				s->alerts().emplace_alert<session_error_alert>(error_code(), e.what());
 			} catch (...) {
-				m_impl->alerts().emplace_alert<session_error_alert>(error_code(), "unknown error");
+				s->alerts().emplace_alert<session_error_alert>(error_code(), "unknown error");
 			}
 #endif
 		});
@@ -75,58 +77,64 @@ namespace libtorrent {
 	template<typename Fun, typename... Args>
 	void session_handle::sync_call(Fun f, Args&&... a) const
 	{
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+
 		// this is the flag to indicate the call has completed
 		// capture them by pointer to allow everything to be captured by value
 		// and simplify the capture expression
 		bool done = false;
 
 		std::exception_ptr ex;
-		m_impl->get_io_service().dispatch([=, &done, &ex]() mutable
+		s->get_io_service().dispatch([=, &done, &ex]() mutable
 		{
 #ifndef BOOST_NO_EXCEPTIONS
 			try {
 #endif
-				(m_impl->*f)(std::forward<Args>(a)...);
+				(s.get()->*f)(std::forward<Args>(a)...);
 #ifndef BOOST_NO_EXCEPTIONS
 			} catch (...) {
 				ex = std::current_exception();
 			}
 #endif
-			std::unique_lock<std::mutex> l(m_impl->mut);
+			std::unique_lock<std::mutex> l(s->mut);
 			done = true;
-			m_impl->cond.notify_all();
+			s->cond.notify_all();
 		});
 
-		aux::torrent_wait(done, *m_impl);
+		aux::torrent_wait(done, *s);
 		if (ex) std::rethrow_exception(ex);
 	}
 
 	template<typename Ret, typename Fun, typename... Args>
 	Ret session_handle::sync_call_ret(Fun f, Args&&... a) const
 	{
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+
 		// this is the flag to indicate the call has completed
 		// capture them by pointer to allow everything to be captured by value
 		// and simplify the capture expression
 		bool done = false;
 		Ret r;
 		std::exception_ptr ex;
-		m_impl->get_io_service().dispatch([=, &r, &done, &ex]() mutable
+		s->get_io_service().dispatch([=, &r, &done, &ex]() mutable
 		{
 #ifndef BOOST_NO_EXCEPTIONS
 			try {
 #endif
-				r = (m_impl->*f)(std::forward<Args>(a)...);
+				r = (s.get()->*f)(std::forward<Args>(a)...);
 #ifndef BOOST_NO_EXCEPTIONS
 			} catch (...) {
 				ex = std::current_exception();
 			}
 #endif
-			std::unique_lock<std::mutex> l(m_impl->mut);
+			std::unique_lock<std::mutex> l(s->mut);
 			done = true;
-			m_impl->cond.notify_all();
+			s->cond.notify_all();
 		});
 
-		aux::torrent_wait(done, *m_impl);
+		aux::torrent_wait(done, *s);
 		if (ex) std::rethrow_exception(ex);
 		return r;
 	}
@@ -175,7 +183,9 @@ namespace libtorrent {
 
 	io_service& session_handle::get_io_service()
 	{
-		return m_impl->get_io_service();
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+		return s->get_io_service();
 	}
 
 	torrent_handle session_handle::find_torrent(sha1_hash const& info_hash) const
@@ -1055,17 +1065,23 @@ namespace {
 	// the alerts are const, they may not be deleted by the client
 	void session_handle::pop_alerts(std::vector<alert*>* alerts)
 	{
-		m_impl->pop_alerts(alerts);
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+		s->pop_alerts(alerts);
 	}
 
 	alert* session_handle::wait_for_alert(time_duration max_wait)
 	{
-		return m_impl->wait_for_alert(max_wait);
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+		return s->wait_for_alert(max_wait);
 	}
 
 	void session_handle::set_alert_notify(std::function<void()> const& fun)
 	{
-		m_impl->alerts().set_notify_function(fun);
+		std::shared_ptr<session_impl> s = m_impl.lock();
+		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
+		s->alerts().set_notify_function(fun);
 	}
 
 #ifndef TORRENT_NO_DEPRECATE

src/session_impl.cpp

@@ -890,7 +890,7 @@ namespace aux {
 			m_ses_extensions[plugins_dht_request_idx].push_back(ext);
 		if (features & plugin::alert_feature)
 			m_alerts.add_extension(ext);
-		session_handle h(this);
+		session_handle h(shared_from_this());
 		ext->added(h);
 	}