From 5b3a730b1f4a85770875480e4ed53408e6feb573 Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 15 Nov 2016 21:57:44 -0500 Subject: [PATCH] steps towards removing disk_io_job from disk_interface --- include/libtorrent/torrent.hpp | 6 ++- src/disk_io_thread.cpp | 33 ++++++------ src/peer_connection.cpp | 43 ++++++++++++++-- src/torrent.cpp | 92 ++++++++++------------------------ 4 files changed, 88 insertions(+), 86 deletions(-) diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index ab0632be6..fa8f954a7 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -449,7 +449,11 @@ namespace libtorrent std::string resolve_filename(int file) const; void handle_exception(); - void handle_disk_error(disk_io_job const* j, peer_connection* c = 0); + + enum class disk_class { none, write }; + void handle_disk_error(string_view job_name + , storage_error const& error, peer_connection* c = nullptr + , disk_class rw = disk_class::none); void clear_error(); void set_error(error_code const& ec, int file); diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 8e35b9aa1..4bdeac6c8 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -773,7 +773,7 @@ namespace libtorrent { disk_io_job* j = src.pop_front(); TORRENT_ASSERT((j->flags & disk_io_job::in_progress) || !j->storage); - j->ret = -1; + j->ret = disk_interface::fatal_disk_error; j->error = e; dst.push_back(j); } @@ -1101,25 +1101,26 @@ namespace libtorrent } catch (boost::system::system_error const& err) { - ret = -1; + ret = disk_interface::fatal_disk_error; j->error.ec = err.code(); j->error.operation = storage_error::exception; } catch (std::bad_alloc const&) { - ret = -1; + ret = disk_interface::fatal_disk_error; j->error.ec = errors::no_memory; j->error.operation = storage_error::exception; } catch (std::exception const&) { - ret = -1; + ret = disk_interface::fatal_disk_error; j->error.ec = boost::asio::error::fault; j->error.operation = storage_error::exception; } // note that -2 errors are OK - TORRENT_ASSERT(ret != -1 || (j->error.ec && j->error.operation != 0)); + TORRENT_ASSERT(ret != disk_interface::fatal_disk_error + || (j->error.ec && j->error.operation != 0)); m_stats_counters.inc_stats_counter(counters::num_running_disk_jobs, -1); @@ -1178,7 +1179,7 @@ namespace libtorrent { j->error.ec = error::no_memory; j->error.operation = storage_error::alloc_cache_piece; - return -1; + return disk_interface::fatal_disk_error; } time_point start_time = clock_type::now(); @@ -1464,7 +1465,7 @@ namespace libtorrent TORRENT_ASSERT(pe->blocks[j->d.io.offset / 16 / 1024].buf != nullptr); j->error.ec = error::operation_aborted; j->error.operation = storage_error::write; - return -1; + return disk_interface::fatal_disk_error; } pe = m_disk_cache.add_dirty_block(j); @@ -1598,7 +1599,7 @@ namespace libtorrent if (pe == nullptr) { - j->ret = -1; + j->ret = disk_interface::fatal_disk_error; j->error.ec = error::no_memory; j->error.operation = storage_error::read; return 0; @@ -2126,7 +2127,7 @@ namespace libtorrent sha1_hash piece_hash = h.final(); std::memcpy(j->d.piece_hash, piece_hash.data(), 20); - return ret >= 0 ? 0 : -1; + return ret >= 0 ? 0 : disk_interface::fatal_disk_error; } int disk_io_thread::do_hash(disk_io_job* j, jobqueue_t& /* completed_jobs */ ) @@ -2187,7 +2188,7 @@ namespace libtorrent { j->error.ec = error::no_memory; j->error.operation = storage_error::alloc_cache_piece; - return -1; + return disk_interface::fatal_disk_error; } if (pe->hashing) @@ -2286,7 +2287,7 @@ namespace libtorrent j->error.ec = errors::no_memory; j->error.operation = storage_error::alloc_cache_piece; - return -1; + return disk_interface::fatal_disk_error; } DLOG("do_hash: reading (piece: %d block: %d)\n", int(pe->piece), i); @@ -2309,7 +2310,7 @@ namespace libtorrent // of this file if (ret != iov.iov_len) { - ret = -1; + ret = disk_interface::fatal_disk_error; j->error.ec = boost::asio::error::eof; j->error.operation = storage_error::read; m_disk_cache.free_buffer(static_cast(iov.iov_base)); @@ -2392,7 +2393,7 @@ namespace libtorrent l.unlock(); j->storage->release_files(j->error); - return j->error ? -1 : 0; + return j->error ? disk_interface::fatal_disk_error : 0; } int disk_io_thread::do_delete_files(disk_io_job* j, jobqueue_t& completed_jobs) @@ -2409,7 +2410,7 @@ namespace libtorrent l.unlock(); j->storage->delete_files(j->buffer.delete_options, j->error); - return j->error ? -1 : 0; + return j->error ? disk_interface::fatal_disk_error : 0; } int disk_io_thread::do_check_fastresume(disk_io_job* j, jobqueue_t& /* completed_jobs */ ) @@ -2485,7 +2486,7 @@ namespace libtorrent // if files need to be closed, that's the storage's responsibility j->storage->rename_file(j->piece, j->buffer.string , j->error); - return j->error ? -1 : 0; + return j->error ? disk_interface::fatal_disk_error : disk_interface::no_error; } int disk_io_thread::do_stop_torrent(disk_io_job* j, jobqueue_t& completed_jobs) @@ -2503,7 +2504,7 @@ namespace libtorrent m_disk_cache.release_memory(); j->storage->release_files(j->error); - return j->error ? -1 : 0; + return j->error ? disk_interface::fatal_disk_error : disk_interface::no_error; } namespace { diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index 0196f6ba6..25138d7ec 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -2998,12 +2998,47 @@ namespace libtorrent // to allow to receive more data setup_receive(); - piece_block block_finished(p.piece, p.start / t->block_size()); + piece_block const block_finished(p.piece, p.start / t->block_size()); if (j->ret < 0) { + // we failed to write j->piece to disk tell the piece picker + // this will block any other peer from issuing requests + // to this piece, until we've cleared it. + if (j->error.ec == boost::asio::error::operation_aborted) + { + if (t->has_picker()) + t->picker().mark_as_canceled(block_finished, nullptr); + } + else + { + // if any other peer has a busy request to this block, we need + // to cancel it too + t->cancel_block(block_finished); + if (t->has_picker()) + t->picker().write_failed(block_finished); + + if (t->has_storage()) + { + // when this returns, all outstanding jobs to the + // piece are done, and we can restore it, allowing + // new requests to it + m_disk_thread.async_clear_piece(&t->storage(), j->piece + , std::bind(&torrent::on_piece_fail_sync, t, _1, block_finished)); + } + else + { + // is m_abort true? if so, we should probably just + // exit this function early, no need to keep the picker + // state up-to-date, right? + disk_io_job sj; + sj.piece = j->piece; + t->on_piece_fail_sync(&sj, block_finished); + } + } + t->update_gauge(); // handle_disk_error may disconnect us - t->handle_disk_error(j, this); + t->handle_disk_error("write", j->error, this, torrent::disk_class::write); return; } @@ -5146,7 +5181,7 @@ namespace libtorrent if (j->error) { - t->handle_disk_error(j, this); + t->handle_disk_error("hash", j->error, this); t->leave_seed_mode(false); return; } @@ -5261,7 +5296,7 @@ namespace libtorrent if (j->ret != r.length) { // handle_disk_error may disconnect us - t->handle_disk_error(j, this); + t->handle_disk_error("read", j->error, this); return; } diff --git a/src/torrent.cpp b/src/torrent.cpp index f000d53c2..bbe808ee7 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -1032,89 +1032,51 @@ namespace libtorrent } } - void torrent::handle_disk_error(disk_io_job const* j, peer_connection* c) + void torrent::handle_disk_error(string_view job_name + , storage_error const& error + , peer_connection* c + , disk_class rw) { TORRENT_ASSERT(is_single_thread()); - if (!j->error) return; + TORRENT_ASSERT(error); #ifndef TORRENT_DISABLE_LOGGING if (should_log()) { - debug_log("disk error: (%d) %s [%s : %s] in file: %s" - , j->error.ec.value(), j->error.ec.message().c_str() - , job_name(j->action), j->error.operation_str() - , resolve_filename(j->error.file).c_str()); + debug_log("disk error: (%d) %s [%*s : %s] in file: %s" + , error.ec.value(), error.ec.message().c_str() + , int(job_name.size()), job_name.data(), error.operation_str() + , resolve_filename(error.file).c_str()); } #endif - if (j->action == disk_io_job::write) - { - piece_block block_finished(j->piece, j->d.io.offset / block_size()); - - // we failed to write j->piece to disk tell the piece picker - // this will block any other peer from issuing requests - // to this piece, until we've cleared it. - if (j->error.ec == boost::asio::error::operation_aborted) - { - if (has_picker()) - picker().mark_as_canceled(block_finished, nullptr); - } - else - { - // if any other peer has a busy request to this block, we need - // to cancel it too - cancel_block(block_finished); - if (has_picker()) - picker().write_failed(block_finished); - - if (m_storage) - { - // when this returns, all outstanding jobs to the - // piece are done, and we can restore it, allowing - // new requests to it - m_ses.disk_thread().async_clear_piece(m_storage.get(), j->piece - , std::bind(&torrent::on_piece_fail_sync, shared_from_this(), _1, block_finished)); - } - else - { - // is m_abort true? if so, we should probably just - // exit this function early, no need to keep the picker - // state up-to-date, right? - disk_io_job sj; - sj.piece = j->piece; - on_piece_fail_sync(&sj, block_finished); - } - } - update_gauge(); - } - - if (j->error.ec == boost::system::errc::not_enough_memory) + if (error.ec == boost::system::errc::not_enough_memory) { if (alerts().should_post()) - alerts().emplace_alert(j->error.ec - , resolve_filename(j->error.file), j->error.operation_str(), get_handle()); + alerts().emplace_alert(error.ec + , resolve_filename(error.file), error.operation_str(), get_handle()); if (c) c->disconnect(errors::no_memory, op_file); return; } - if (j->error.ec == boost::asio::error::operation_aborted) return; + if (error.ec == boost::asio::error::operation_aborted) return; // notify the user of the error if (alerts().should_post()) - alerts().emplace_alert(j->error.ec - , resolve_filename(j->error.file), j->error.operation_str(), get_handle()); + alerts().emplace_alert(error.ec + , resolve_filename(error.file), error.operation_str(), get_handle()); // if a write operation failed, and future writes are likely to // fail, while reads may succeed, just set the torrent to upload mode // if we make an incorrect assumption here, it's not the end of the // world, if we ever issue a read request and it fails as well, we // won't get in here and we'll actually end up pausing the torrent - if (j->action == disk_io_job::write - && (j->error.ec == boost::system::errc::read_only_file_system - || j->error.ec == boost::system::errc::permission_denied - || j->error.ec == boost::system::errc::operation_not_permitted - || j->error.ec == boost::system::errc::no_space_on_device - || j->error.ec == boost::system::errc::file_too_large)) + if (rw == disk_class::write + && (error.ec == boost::system::errc::read_only_file_system + || error.ec == boost::system::errc::permission_denied + || error.ec == boost::system::errc::operation_not_permitted + || error.ec == boost::system::errc::no_space_on_device + || error.ec == boost::system::errc::file_too_large)) { // if we failed to write, stop downloading and just // keep seeding. @@ -1128,7 +1090,7 @@ namespace libtorrent } // put the torrent in an error-state - set_error(j->error.ec, j->error.file); + set_error(error.ec, error.file); // if the error appears to be more serious than a full disk, just pause the torrent pause(); @@ -1178,7 +1140,7 @@ namespace libtorrent { rp->fail = true; rp->error = j->error.ec; - handle_disk_error(j); + handle_disk_error("read", j->error); } else { @@ -1334,7 +1296,7 @@ namespace libtorrent if (j->ret == -1) { - handle_disk_error(j); + handle_disk_error("write", j->error); return; } @@ -1988,7 +1950,7 @@ namespace libtorrent { TORRENT_ASSERT(m_outstanding_check_files == false); m_add_torrent_params.reset(); - handle_disk_error(j); + handle_disk_error("check_resume_data", j->error); auto_managed(false); pause(); set_state(torrent_status::checking_files); @@ -2240,7 +2202,7 @@ namespace libtorrent if (j->ret == disk_interface::fatal_disk_error) { - handle_disk_error(j); + handle_disk_error("force_recheck", j->error); return; } if (j->ret == 0) @@ -3708,7 +3670,7 @@ namespace libtorrent } else if (ret == -1) { - handle_disk_error(j); + handle_disk_error("piece_verified", j->error); } else {