fixed crash when shutting down while checking a torrent

ChangeLog

@@ -46,6 +46,7 @@ release 0.14.4
 	  in an error state which would pause the torrent
 	* fixed case when move_storage() would fail. Added a new alert
 	  to be posted when it does
+	* fixed crash bug when shutting down while checking a torrent
 
 release 0.14.3
 

include/libtorrent/disk_io_thread.hpp

@@ -337,6 +337,18 @@ namespace libtorrent
 
 		io_service& m_ios;
 
+		// this keeps the io_service::run() call blocked from
+		// returning. When shutting down, it's possible that
+		// the event queue is drained before the disk_io_thread
+		// has posted its last callback. When this happens, the
+		// io_service will have a pending callback from the
+		// disk_io_thread, but the event loop is not running.
+		// this means that the event is destructed after the
+		// disk_io_thread. If the event refers to a disk buffer
+		// it will try to free it, but the buffer pool won't
+		// exist anymore, and crash. This prevents that.
+		boost::optional<asio::io_service::work> m_work;
+
 		// thread for performing blocking disk io operations
 		boost::thread m_disk_io_thread;
 	};

src/disk_io_thread.cpp

@@ -238,6 +238,7 @@ namespace libtorrent
 		, m_abort(false)
 		, m_queue_buffer_size(0)
 		, m_ios(ios)
+		, m_work(io_service::work(m_ios))
 		, m_disk_io_thread(boost::ref(*this))
 	{
 #ifdef TORRENT_DISK_STATS
@@ -974,6 +975,7 @@ namespace libtorrent
 	void disk_io_thread::add_job(disk_io_job const& j
 		, boost::function<void(int, disk_io_job const&)> const& f)
 	{
+		TORRENT_ASSERT(!m_abort);
 		TORRENT_ASSERT(!j.callback);
 		TORRENT_ASSERT(j.storage
 			|| j.action == disk_io_job::abort_thread
@@ -1090,6 +1092,9 @@ namespace libtorrent
 					free_piece(*i, l);
 				m_pieces.clear();
 				m_read_pieces.clear();
+				// release the io_service to allow the run() call to return
+				// we do this once we stop posting new callbacks to it.
+				m_work.reset();
 				return;
 			}
 
@@ -1170,7 +1175,6 @@ namespace libtorrent
 					m_log << log_time() << " abort_thread " << std::endl;
 #endif
 					mutex_t::scoped_lock jl(m_queue_mutex);
-					m_abort = true;
 
 					for (std::list<disk_io_job>::iterator i = m_jobs.begin();
 							i != m_jobs.end();)
@@ -1189,6 +1193,8 @@ namespace libtorrent
 						}
 						++i;
 					}
+
+					m_abort = true;
 					break;
 				}
 				case disk_io_job::read_and_hash:

test/test_storage.cpp

@@ -64,7 +64,7 @@ void on_read_piece(int ret, disk_io_job const& j, char const* data, int size)
 	if (ret > 0) TEST_CHECK(std::equal(j.buffer, j.buffer + ret, data));
 }
 
-void on_check_resume_data(int ret, disk_io_job const& j)
+void on_check_resume_data(int ret, disk_io_job const& j, bool* done)
 {
 	std::cerr << "on_check_resume_data ret: " << ret;
 	switch (ret)
@@ -75,19 +75,20 @@ void on_check_resume_data(int ret, disk_io_job const& j)
 			<< " file: " << j.error_file << std::endl; break;
 		case -3: std::cerr << " aborted" << std::endl; break;
 	}
+	*done = true;
 }
 
-void on_check_files(int ret, disk_io_job const& j)
+void on_check_files(int ret, disk_io_job const& j, bool* done)
 {
 	std::cerr << "on_check_files ret: " << ret;
 
 	switch (ret)
 	{
-		case 0: std::cerr << " done" << std::endl; break;
+		case 0: std::cerr << " done" << std::endl; *done = true; break;
 		case -1: std::cerr << " current slot: " << j.piece << " have: " << j.offset << std::endl; break;
 		case -2: std::cerr << " disk error: " << j.str
-			<< " file: " << j.error_file << std::endl; break;
-		case -3: std::cerr << " aborted" << std::endl; break;
+			<< " file: " << j.error_file << std::endl; *done = true; break;
+		case -3: std::cerr << " aborted" << std::endl; *done = true; break;
 	}
 }
 
@@ -187,19 +188,23 @@ void run_storage_tests(boost::intrusive_ptr<torrent_info> info
 	boost::mutex lock;
 
 	error_code ec;
+	bool done = false;
 	lazy_entry frd;
-	pm->async_check_fastresume(&frd, &on_check_resume_data);
+	pm->async_check_fastresume(&frd, boost::bind(&on_check_resume_data, _1, _2, &done));
 	ios.reset();
-	ios.run(ec);
-
-	pm->async_check_files(&on_check_files);
-	for (int i = 0; i < 4; ++i)
+	while (!done)
+	{
+		ios.reset();
+		ios.run_one(ec);
+	}
+
+	done = false;
+	pm->async_check_files(boost::bind(&on_check_files, _1, _2, &done));
+	while (!done)
 	{
 		ios.reset();
 		ios.run_one(ec);
 	}
-	ios.reset();
-	ios.poll(ec);
 
 	// test rename_file
 	remove(test_path / "part0");
@@ -260,7 +265,8 @@ void run_storage_tests(boost::intrusive_ptr<torrent_info> info
 	TEST_CHECK(!exists(test_path / "part0"));	
 	TEST_CHECK(exists(test_path / "temp_storage/test1.tmp"));
 
-	ios.run(ec);
+	ios.reset();
+	ios.poll(ec);
 
 	io.join();
 	remove_all(test_path / "temp_storage2");
@@ -367,13 +373,18 @@ void test_check_files(path const& test_path
 	boost::mutex lock;
 
 	error_code ec;
+	bool done = false;
 	lazy_entry frd;
-	pm->async_check_fastresume(&frd, &on_check_resume_data);
+	pm->async_check_fastresume(&frd, boost::bind(&on_check_resume_data, _1, _2, &done));
 	ios.reset();
-	ios.run(ec);
+	while (!done)
+	{
+		ios.reset();
+		ios.run_one(ec);
+	}
 
 	bool pieces[4] = {false, false, false, false};
-	bool done = false;
+	done = false;
 
 	pm->async_check_files(bind(&check_files_fill_array, _1, _2, pieces, &done));
 	while (!done)