diff --git a/include/libtorrent/aux_/block_cache_reference.hpp b/include/libtorrent/aux_/block_cache_reference.hpp index 63b6a9ca3..5314207fd 100644 --- a/include/libtorrent/aux_/block_cache_reference.hpp +++ b/include/libtorrent/aux_/block_cache_reference.hpp @@ -34,15 +34,19 @@ POSSIBILITY OF SUCH DAMAGE. #define TORRENT_BLOCK_CACHE_REFERENCE_HPP #include "libtorrent/units.hpp" +#include "libtorrent/storage_defs.hpp" namespace libtorrent { namespace aux { struct block_cache_reference { - void* storage; - piece_index_t piece; - int block; + storage_index_t storage; + std::uint32_t cookie; + + // if the cookie is set to this value, it doesn't refer to anything in the + // cache (and the buffer is mutable) + static std::uint32_t const none = 0xffffffffu; }; } diff --git a/include/libtorrent/block_cache.hpp b/include/libtorrent/block_cache.hpp index d70d1fc78..67187dab9 100644 --- a/include/libtorrent/block_cache.hpp +++ b/include/libtorrent/block_cache.hpp @@ -344,7 +344,7 @@ namespace libtorrent int pad_job(disk_io_job const* j, int blocks_in_piece , int read_ahead) const; - void reclaim_block(aux::block_cache_reference const& ref); + void reclaim_block(storage_interface* st, aux::block_cache_reference const& ref); // returns a range of all pieces. This might be a very // long list, use carefully @@ -406,7 +406,6 @@ namespace libtorrent // looks for this piece in the cache. If it's there, returns a pointer // to it, otherwise 0. - cached_piece_entry* find_piece(aux::block_cache_reference const& ref); cached_piece_entry* find_piece(disk_io_job const* j); cached_piece_entry* find_piece(storage_interface* st, piece_index_t piece); diff --git a/include/libtorrent/chained_buffer.hpp b/include/libtorrent/chained_buffer.hpp index decd0d441..ec406da0e 100644 --- a/include/libtorrent/chained_buffer.hpp +++ b/include/libtorrent/chained_buffer.hpp @@ -111,7 +111,7 @@ namespace libtorrent #if TORRENT_CPP98_DEQUE move_construct_holder_fun move_holder; #endif - std::aligned_storage<32>::type holder; + std::aligned_storage<24>::type holder; char* buf; // the first byte of the buffer int size; // the total size of the buffer int used_size; // this is the number of bytes to send/receive diff --git a/include/libtorrent/disk_buffer_holder.hpp b/include/libtorrent/disk_buffer_holder.hpp index 59421b722..cef10fa73 100644 --- a/include/libtorrent/disk_buffer_holder.hpp +++ b/include/libtorrent/disk_buffer_holder.hpp @@ -103,7 +103,7 @@ namespace libtorrent } // if this returns true, the buffer may not be modified in place - bool is_mutable() const noexcept { return m_ref.storage == nullptr; } + bool is_mutable() const noexcept { return m_ref.cookie == aux::block_cache_reference::none; } // implicitly convertible to true if the object is currently holding a // buffer diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index 55209cf2a..608ee2faf 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -199,7 +199,7 @@ namespace libtorrent // operations until all outstanding jobs have completed. // at that point, the m_blocked_jobs are issued // the count is the number of fence job currently in the queue - int m_has_fence; + int m_has_fence = 0; // when there's a fence up, jobs are queued up in here // until the fence is lowered @@ -209,7 +209,7 @@ namespace libtorrent // to this torrent, currently pending, hanging off of // cached_piece_entry objects. This is used to determine // when the fence can be lowered - std::atomic m_outstanding_jobs; + std::atomic m_outstanding_jobs{0}; // must be held when accessing m_has_fence and // m_blocked_jobs @@ -449,6 +449,13 @@ namespace libtorrent storage_index_t storage_index() const { return m_storage_index; } void set_storage_index(storage_index_t st) { m_storage_index = st; } + + int dec_refcount() + { + TORRENT_ASSERT(m_references > 0); + return m_references--; + } + void inc_refcount() { ++m_references; } private: bool m_need_tick = false; @@ -459,10 +466,13 @@ namespace libtorrent // torrent. This shared_ptr is here only // to keep the torrent object alive until // the storage_interface destructs. This is because - // the torrent_info object is owned by the torrent. + // the file_storage object is owned by the torrent. std::shared_ptr m_torrent; storage_index_t m_storage_index; + + // the number of block_cache_reference objects referencing this storage + std::atomic m_references{1}; }; // The default implementation of storage_interface. Behaves as a normal diff --git a/src/block_cache.cpp b/src/block_cache.cpp index 1dc161dc9..8047fa01b 100644 --- a/src/block_cache.cpp +++ b/src/block_cache.cpp @@ -1744,10 +1744,12 @@ int block_cache::copy_from_piece(cached_piece_entry* const pe // make sure it didn't wrap TORRENT_PIECE_ASSERT(pe->refcount > 0, pe); - j->d.io.ref.storage = j->storage.get(); - j->d.io.ref.piece = pe->piece; - j->d.io.ref.block = start_block; + int const blocks_per_piece = (j->storage->files()->piece_length() + block_size() - 1) / block_size(); + j->d.io.ref.storage = j->storage->storage_index(); + j->d.io.ref.cookie = static_cast(pe->piece) * blocks_per_piece + start_block; j->buffer.disk_block = bl.buf + (j->d.io.offset & (block_size()-1)); + j->storage->inc_refcount(); + ++m_send_buffer_blocks; return j->d.io.buffer_size; } @@ -1784,16 +1786,20 @@ int block_cache::copy_from_piece(cached_piece_entry* const pe return j->d.io.buffer_size; } -void block_cache::reclaim_block(aux::block_cache_reference const& ref) +void block_cache::reclaim_block(storage_interface* st, aux::block_cache_reference const& ref) { - cached_piece_entry* pe = find_piece(ref); + int const blocks_per_piece = (st->files()->piece_length() + block_size() - 1) / block_size(); + piece_index_t const piece(ref.cookie / blocks_per_piece); + int const block(ref.cookie % blocks_per_piece); + + cached_piece_entry* pe = find_piece(st, piece); TORRENT_ASSERT(pe); if (pe == nullptr) return; TORRENT_PIECE_ASSERT(pe->in_use, pe); - TORRENT_PIECE_ASSERT(pe->blocks[ref.block].buf, pe); - dec_block_refcount(pe, ref.block, block_cache::ref_reading); + TORRENT_PIECE_ASSERT(pe->blocks[block].buf, pe); + dec_block_refcount(pe, block, block_cache::ref_reading); TORRENT_PIECE_ASSERT(m_send_buffer_blocks > 0, pe); --m_send_buffer_blocks; @@ -1822,11 +1828,6 @@ bool block_cache::maybe_free_piece(cached_piece_entry* pe) return true; } -cached_piece_entry* block_cache::find_piece(aux::block_cache_reference const& ref) -{ - return find_piece(static_cast(ref.storage), ref.piece); -} - cached_piece_entry* block_cache::find_piece(disk_io_job const* j) { return find_piece(j->storage.get(), j->piece); diff --git a/src/disk_buffer_holder.cpp b/src/disk_buffer_holder.cpp index 5228cad81..183498c48 100644 --- a/src/disk_buffer_holder.cpp +++ b/src/disk_buffer_holder.cpp @@ -40,9 +40,8 @@ namespace libtorrent disk_buffer_holder::disk_buffer_holder(buffer_allocator_interface& alloc, char* buf) noexcept : m_allocator(&alloc), m_buf(buf) { - m_ref.storage = nullptr; - m_ref.piece = piece_index_t(-1); - m_ref.block = -1; + m_ref.storage = storage_index_t{0}; + m_ref.cookie = aux::block_cache_reference::none; } disk_buffer_holder& disk_buffer_holder::operator=(disk_buffer_holder&& h) noexcept @@ -61,44 +60,32 @@ namespace libtorrent disk_buffer_holder::disk_buffer_holder(buffer_allocator_interface& alloc , aux::block_cache_reference const& ref, char* buf) noexcept : m_allocator(&alloc), m_buf(buf), m_ref(ref) - { - TORRENT_ASSERT(m_ref.storage == nullptr || m_ref.piece >= piece_index_t(0)); - TORRENT_ASSERT(m_ref.storage == nullptr || m_ref.block >= 0); - TORRENT_ASSERT(m_ref.storage == nullptr - || m_ref.piece < static_cast(m_ref.storage)->files()->end_piece()); - TORRENT_ASSERT(m_ref.storage == nullptr - || m_ref.block <= static_cast(m_ref.storage)->files()->piece_length() / 0x4000); - } + {} void disk_buffer_holder::reset(aux::block_cache_reference const& ref, char* buf) { - if (m_ref.storage) m_allocator->reclaim_blocks(m_ref); + if (m_ref.cookie != aux::block_cache_reference::none) m_allocator->reclaim_blocks(m_ref); else if (m_buf) m_allocator->free_disk_buffer(m_buf); m_buf = buf; m_ref = ref; - - TORRENT_ASSERT(m_ref.piece >= piece_index_t(0)); - TORRENT_ASSERT(m_ref.storage != nullptr); - TORRENT_ASSERT(m_ref.block >= 0); - TORRENT_ASSERT(m_ref.piece < static_cast(m_ref.storage)->files()->end_piece()); - TORRENT_ASSERT(m_ref.block <= static_cast(m_ref.storage)->files()->piece_length() / 0x4000); } void disk_buffer_holder::reset(char* const buf) { - if (m_ref.storage) m_allocator->reclaim_blocks(m_ref); + if (m_ref.cookie != aux::block_cache_reference::none) m_allocator->reclaim_blocks(m_ref); else if (m_buf) m_allocator->free_disk_buffer(m_buf); m_buf = buf; - m_ref.storage = nullptr; + m_ref.cookie = aux::block_cache_reference::none; } char* disk_buffer_holder::release() noexcept { char* ret = m_buf; m_buf = nullptr; - m_ref.storage = nullptr; + m_ref.cookie = aux::block_cache_reference::none; return ret; } disk_buffer_holder::~disk_buffer_holder() { reset(); } } + diff --git a/src/disk_io_job.cpp b/src/disk_io_job.cpp index 4c8e4132d..b7b2b4065 100644 --- a/src/disk_io_job.cpp +++ b/src/disk_io_job.cpp @@ -105,9 +105,8 @@ namespace libtorrent buffer.disk_block = nullptr; d.io.offset = 0; d.io.buffer_size = 0; - d.io.ref.storage = nullptr; - d.io.ref.piece = piece_index_t{0}; - d.io.ref.block = 0; + d.io.ref.storage = storage_index_t{0}; + d.io.ref.cookie = aux::block_cache_reference::none; } disk_io_job::~disk_io_job() diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 3eda477c9..3910e3eb4 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -232,7 +232,8 @@ namespace libtorrent void disk_io_thread::remove_torrent(storage_index_t const idx) { - m_torrents[idx].reset(); + auto& pos = m_torrents[idx]; + if (pos->dec_refcount() == 0) pos.reset(); } disk_io_thread::~disk_io_thread() @@ -279,8 +280,10 @@ namespace libtorrent std::unique_lock l(m_cache_mutex); for (auto ref : refs) { - TORRENT_ASSERT(ref.storage); - m_disk_cache.reclaim_block(ref); + auto& pos = m_torrents[ref.storage]; + storage_interface* st = pos.get(); + m_disk_cache.reclaim_block(st, ref); + if (st->dec_refcount() == 0) pos.reset(); } } diff --git a/src/storage.cpp b/src/storage.cpp index fe65b7ee0..583b8a4dc 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -1477,10 +1477,7 @@ namespace libtorrent // ====== disk_job_fence implementation ======== - disk_job_fence::disk_job_fence() - : m_has_fence(0) - , m_outstanding_jobs(0) - {} + disk_job_fence::disk_job_fence() {} int disk_job_fence::job_complete(disk_io_job* j, tailqueue& jobs) { diff --git a/src/torrent.cpp b/src/torrent.cpp index bbe4c90b6..687c7bfad 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -4322,7 +4322,13 @@ namespace libtorrent alerts().emplace_alert(get_handle()); } - m_storage.reset(); + // if this torrent still has connections, they may have buffers to return + // to the disk storage, in which case we cannot destroy the disk storage + // yet. wait until the last peer connection is removed + if (m_connections.empty()) + { + m_storage.reset(); + } // TODO: 2 abort lookups this torrent has made via the // session host resolver interface @@ -7127,9 +7133,6 @@ namespace libtorrent void torrent::disconnect_all(error_code const& ec, operation_t op) { -// doesn't work with the m_paused -> m_num_peers == 0 condition -// INVARIANT_CHECK; - while (!m_connections.empty()) { peer_connection* p = *m_connections.begin(); diff --git a/test/test_block_cache.cpp b/test/test_block_cache.cpp index 3ee6f4053..ac2476e80 100644 --- a/test/test_block_cache.cpp +++ b/test/test_block_cache.cpp @@ -134,9 +134,9 @@ static void nop() {} ret = bc.try_read(&rj) #define RETURN_BUFFER \ - if (rj.d.io.ref.storage) bc.reclaim_block(rj.d.io.ref); \ + if (rj.d.io.ref.cookie != aux::block_cache_reference::none) bc.reclaim_block(pm.get(), rj.d.io.ref); \ else if (rj.buffer.disk_block) bc.free_buffer(rj.buffer.disk_block); \ - rj.d.io.ref.storage = 0 + rj.d.io.ref.cookie = aux::block_cache_reference::none #define FLUSH(flushing) \ for (int i = 0; i < int(sizeof(flushing)/sizeof((flushing)[0])); ++i) \