Fix arguments of dispatched functions cannot be actually moved #7873

glassez · 2025-02-15T16:30:24Z

It seems that move-only arguments were never used, so the problem went unnoticed. I discovered it when I was experimenting with boost::promise.

glassez · 2025-02-15T16:47:44Z

Oh, damn it! libtorrent still supports C++17...

src/session_handle.cpp

src/torrent_handle.cpp

src/session_handle.cpp

src/torrent_handle.cpp

src/session_handle.cpp

src/torrent_handle.cpp

glassez · 2025-02-16T04:45:25Z

src/session_handle.cpp

@@ -92,12 +93,12 @@ namespace libtorrent {
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([=](auto&&... args) mutable


Note that trick with std::bind isn't required since c++20. It can be rewritten as the following:

dispatch(s->get_context(), [=, ...args = std::forward<Args>(a)]() mutable { // do something });

glassez · 2025-02-16T04:51:56Z

test/test_storage.cpp

@@ -928,11 +928,12 @@ void test_fastresume(bool const test_deprecated)
 		p.storage_mode = storage_mode_sparse;
 		error_code ignore;
 		torrent_handle h = ses.add_torrent(std::move(p), ignore);
-		TEST_CHECK(exists(combine_path(p.save_path, "temporary")));


It is noteworthy that this "use-after-move" bug also went unnoticed, as p was not actually moved.

src/session_handle.cpp

@@ -92,12 +93,12 @@
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable


src/session_handle.cpp

@@ -92,12 +93,12 @@
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable


src/session_handle.cpp

@@ -92,12 +93,12 @@
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable


src/session_handle.cpp

@@ -154,12 +155,12 @@
 		bool done = false;
 		Ret r;
 		std::exception_ptr ex;
-		dispatch(s->get_context(), [=, &r, &done, &ex]() mutable
+		dispatch(s->get_context(), std::bind([s, f, &r, &done, &ex](auto&&... args) mutable


src/session_handle.cpp

@@ -154,12 +155,12 @@
 		bool done = false;
 		Ret r;
 		std::exception_ptr ex;
-		dispatch(s->get_context(), [=, &r, &done, &ex]() mutable
+		dispatch(s->get_context(), std::bind([s, f, &r, &done, &ex](auto&&... args) mutable


src/torrent_handle.cpp

@@ -122,12 +123,12 @@
 		std::shared_ptr<torrent> t = m_torrent.lock();
 		if (!t) aux::throw_ex<system_error>(errors::invalid_torrent_handle);
 		auto& ses = static_cast<session_impl&>(t->session());
-		dispatch(ses.get_context(), [=,&ses] ()
+		dispatch(ses.get_context(), std::bind([t, f, &ses](auto&&... args) mutable


src/torrent_handle.cpp

@@ -122,12 +123,12 @@
 		std::shared_ptr<torrent> t = m_torrent.lock();
 		if (!t) aux::throw_ex<system_error>(errors::invalid_torrent_handle);
 		auto& ses = static_cast<session_impl&>(t->session());
-		dispatch(ses.get_context(), [=,&ses] ()
+		dispatch(ses.get_context(), std::bind([t, f, &ses](auto&&... args) mutable


src/torrent_handle.cpp

@@ -190,12 +191,12 @@
 		bool done = false;

 		std::exception_ptr ex;
-		dispatch(ses.get_context(), [=,&r,&done,&ses,&ex] ()
+		dispatch(ses.get_context(), std::bind([t, f, &r, &done, &ses, &ex](auto&&... args) mutable


src/torrent_handle.cpp

@@ -190,12 +191,12 @@
 		bool done = false;

 		std::exception_ptr ex;
-		dispatch(ses.get_context(), [=,&r,&done,&ses,&ex] ()
+		dispatch(ses.get_context(), std::bind([t, f, &r, &done, &ses, &ex](auto&&... args) mutable


src/torrent_handle.cpp

@@ -190,12 +191,12 @@
 		bool done = false;

 		std::exception_ptr ex;
-		dispatch(ses.get_context(), [=,&r,&done,&ses,&ex] ()
+		dispatch(ses.get_context(), std::bind([t, f, &r, &done, &ses, &ex](auto&&... args) mutable


Chocobo1 · 2025-02-17T07:06:22Z

src/session_handle.cpp

@@ -92,12 +93,12 @@ namespace libtorrent {
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable


It would be beneficial to move function f into the lambda too if possible.

It would be beneficial to move function f into the lambda too if possible.

Generally, yes. But in this case, it is intended only for pointer to member function, so moving it will not add any advantages.

I see. So it is a pointer rather than a function object...

Chocobo1 · 2025-02-17T07:08:27Z

src/session_handle.cpp

@@ -92,12 +93,12 @@ namespace libtorrent {
 	{
 		std::shared_ptr<session_impl> s = m_impl.lock();
 		if (!s) aux::throw_ex<system_error>(errors::invalid_session_handle);
-		dispatch(s->get_context(), [=]() mutable
+		dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable


Why not:

Suggested change

dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable

dispatch(s->get_context(), std::bind([s, f](Args&&... args) mutable

Why not:

It is incorrect.

glassez force-pushed the fix-forward-args branch 2 times, most recently from 40be6c3 to 3e1b699 Compare February 15, 2025 17:31

glassez marked this pull request as draft February 15, 2025 17:33

glassez force-pushed the fix-forward-args branch from 3e1b699 to fe0506b Compare February 15, 2025 17:36

github-advanced-security bot found potential problems Feb 15, 2025

View reviewed changes

glassez force-pushed the fix-forward-args branch 2 times, most recently from 145f938 to 1c465b3 Compare February 15, 2025 18:35

glassez commented Feb 15, 2025

View reviewed changes

src/session_handle.cpp Show resolved Hide resolved

src/torrent_handle.cpp Show resolved Hide resolved

glassez commented Feb 15, 2025

View reviewed changes

glassez force-pushed the fix-forward-args branch 3 times, most recently from dd85995 to fceba8d Compare February 16, 2025 04:41

glassez commented Feb 16, 2025

View reviewed changes

glassez marked this pull request as ready for review February 16, 2025 05:00

Fix use-after-move in test_storage

1b72434

glassez force-pushed the fix-forward-args branch from fceba8d to ab89194 Compare February 17, 2025 05:41

github-advanced-security bot found potential problems Feb 17, 2025

View reviewed changes

glassez force-pushed the fix-forward-args branch from ab89194 to 8ec5ee7 Compare February 17, 2025 07:02

Fix arguments of dispatched functions cannot be actually moved

cc0b207

glassez force-pushed the fix-forward-args branch from 8ec5ee7 to cc0b207 Compare February 17, 2025 07:07

Chocobo1 reviewed Feb 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix arguments of dispatched functions cannot be actually moved #7873

Fix arguments of dispatched functions cannot be actually moved #7873

glassez commented Feb 15, 2025

glassez commented Feb 15, 2025

glassez Feb 16, 2025 •

edited

Loading

glassez Feb 16, 2025

Chocobo1 Feb 17, 2025

glassez Feb 17, 2025

Chocobo1 Feb 17, 2025

Chocobo1 Feb 17, 2025

glassez Feb 17, 2025

	dispatch(s->get_context(), std::bind([s, f](auto&&... args) mutable
	dispatch(s->get_context(), std::bind([s, f](Args&&... args) mutable

Fix arguments of dispatched functions cannot be actually moved #7873

Are you sure you want to change the base?

Fix arguments of dispatched functions cannot be actually moved #7873

Conversation

glassez commented Feb 15, 2025

glassez commented Feb 15, 2025

glassez Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

glassez Feb 16, 2025

Choose a reason for hiding this comment

Chocobo1 Feb 17, 2025

Choose a reason for hiding this comment

glassez Feb 17, 2025

Choose a reason for hiding this comment

Chocobo1 Feb 17, 2025

Choose a reason for hiding this comment

Chocobo1 Feb 17, 2025

Choose a reason for hiding this comment

glassez Feb 17, 2025

Choose a reason for hiding this comment

glassez Feb 16, 2025 •

edited

Loading