From f5c7a49d61e58318cf910ba20c571047bd90e3e5 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Thu, 3 Nov 2016 19:39:42 -0400 Subject: [PATCH 1/2] remove incorrect deleted-storages assert and add some more test coverage in test_metadata_extensions (#1286) --- include/libtorrent/block_cache.hpp | 8 ------ simulation/setup_swarm.cpp | 4 +++ simulation/test_metadata_extension.cpp | 35 ++++++++++++++++++++++++- src/block_cache.cpp | 36 -------------------------- src/disk_io_thread.cpp | 3 --- 5 files changed, 38 insertions(+), 48 deletions(-) diff --git a/include/libtorrent/block_cache.hpp b/include/libtorrent/block_cache.hpp index f9de386ed..47a81f7c0 100644 --- a/include/libtorrent/block_cache.hpp +++ b/include/libtorrent/block_cache.hpp @@ -468,10 +468,6 @@ namespace libtorrent int pinned_blocks() const { return m_pinned_blocks; } int read_cache_size() const { return m_read_cache_size; } -#if TORRENT_USE_ASSERTS - void mark_deleted(file_storage const& fs); -#endif - private: // returns number of bytes read on success, -1 on cache miss @@ -536,10 +532,6 @@ namespace libtorrent // the number of blocks with a refcount > 0, i.e. // they may not be evicted int m_pinned_blocks; - -#if TORRENT_USE_ASSERTS - std::vector > m_deleted_storages; -#endif }; } diff --git a/simulation/setup_swarm.cpp b/simulation/setup_swarm.cpp index e30a5337d..a4dc06bfe 100644 --- a/simulation/setup_swarm.cpp +++ b/simulation/setup_swarm.cpp @@ -124,6 +124,7 @@ lt::torrent_status get_status(lt::session& ses) { auto handles = ses.get_torrents(); TEST_EQUAL(handles.size(), 1); + if (handles.empty()) return lt::torrent_status(); auto h = handles[0]; return h.status(); } @@ -132,6 +133,7 @@ bool has_metadata(lt::session& ses) { auto handles = ses.get_torrents(); TEST_EQUAL(handles.size(), 1); + if (handles.empty()) return false; auto h = handles[0]; return h.status().has_metadata; } @@ -140,6 +142,7 @@ bool is_seed(lt::session& ses) { auto handles = ses.get_torrents(); TEST_EQUAL(handles.size(), 1); + if (handles.empty()) return false; auto h = handles[0]; return h.status().is_seeding; } @@ -148,6 +151,7 @@ int completed_pieces(lt::session& ses) { auto handles = ses.get_torrents(); TEST_EQUAL(handles.size(), 1); + if (handles.empty()) return 0; auto h = handles[0]; return h.status().num_pieces; } diff --git a/simulation/test_metadata_extension.cpp b/simulation/test_metadata_extension.cpp index 81dca856a..38444f408 100644 --- a/simulation/test_metadata_extension.cpp +++ b/simulation/test_metadata_extension.cpp @@ -64,7 +64,10 @@ enum flags_t utp = 8, // upload-only mode - upload_only = 16 + upload_only = 16, + + // re-add the torrent after removing + readd = 32 }; void run_metadata_test(int flags) @@ -109,10 +112,21 @@ void run_metadata_test(int flags) { metadata_alerts += 1; + boost::shared_ptr ti = boost::make_shared( + *ses.get_torrents()[0].torrent_file()); + if (flags & disconnect) { ses.remove_torrent(ses.get_torrents()[0]); } + + if (flags & readd) + { + add_torrent_params p = default_add_torrent; + p.ti = ti; + p.save_path = "."; + ses.add_torrent(p); + } } } // terminate @@ -128,6 +142,10 @@ void run_metadata_test(int flags) TEST_ERROR("timeout"); return true; } + if ((flags & disconnect) && metadata_alerts > 0) + { + return true; + } if ((flags & upload_only) && has_metadata(ses)) { // the other peer is in upload mode and should not have sent any @@ -168,3 +186,18 @@ TORRENT_TEST(ut_metadata_upload_only) run_metadata_test(upload_only); } +TORRENT_TEST(ut_metadata_disconnect) +{ + run_metadata_test(disconnect); +} + +TORRENT_TEST(ut_metadata_disconnect_readd) +{ + run_metadata_test(disconnect | readd); +} + +TORRENT_TEST(ut_metadata_upload_only_disconnect_readd) +{ + run_metadata_test(upload_only | disconnect | readd); +} + diff --git a/src/block_cache.cpp b/src/block_cache.cpp index 3bd0757fe..e6188ca6d 100644 --- a/src/block_cache.cpp +++ b/src/block_cache.cpp @@ -376,15 +376,6 @@ int block_cache::try_read(disk_io_job* j, bool expect_no_fail) TORRENT_ASSERT(j->buffer.disk_block == 0); -#if TORRENT_USE_ASSERTS - // we're not allowed to add dirty blocks - // for a deleted storage! - TORRENT_ASSERT(std::find(m_deleted_storages.begin(), m_deleted_storages.end() - , std::make_pair(j->storage->files()->name() - , reinterpret_cast(j->storage->files()))) - == m_deleted_storages.end()); -#endif - cached_piece_entry* p = find_piece(j); int ret = 0; @@ -733,16 +724,6 @@ cached_piece_entry* block_cache::allocate_piece(disk_io_job const* j, int cache_ return p; } -#if TORRENT_USE_ASSERTS -void block_cache::mark_deleted(file_storage const& fs) -{ - m_deleted_storages.push_back(std::make_pair(fs.name() - , reinterpret_cast(&fs))); - if (m_deleted_storages.size() > 100) - m_deleted_storages.erase(m_deleted_storages.begin()); -} -#endif - cached_piece_entry* block_cache::add_dirty_block(disk_io_job* j) { #if !defined TORRENT_DISABLE_POOL_ALLOCATOR @@ -752,15 +733,6 @@ cached_piece_entry* block_cache::add_dirty_block(disk_io_job* j) INVARIANT_CHECK; #endif -#if TORRENT_USE_ASSERTS - // we're not allowed to add dirty blocks - // for a deleted storage! - TORRENT_ASSERT(std::find(m_deleted_storages.begin(), m_deleted_storages.end() - , std::make_pair(j->storage->files()->name() - , static_cast(j->storage->files()))) - == m_deleted_storages.end()); -#endif - TORRENT_ASSERT(j->buffer.disk_block); TORRENT_ASSERT(m_write_cache_size + m_read_cache_size + 1 <= in_use()); @@ -1319,14 +1291,6 @@ void block_cache::insert_blocks(cached_piece_entry* pe, int block, file::iovec_t TORRENT_ASSERT(pe->in_use); TORRENT_PIECE_ASSERT(iov_len > 0, pe); -#if TORRENT_USE_ASSERTS - // we're not allowed to add dirty blocks - // for a deleted storage! - TORRENT_ASSERT(std::find(m_deleted_storages.begin(), m_deleted_storages.end() - , std::make_pair(j->storage->files()->name(), static_cast(j->storage->files()))) - == m_deleted_storages.end()); -#endif - cache_hit(pe, j->requester, (j->flags & disk_io_job::volatile_read) != 0); TORRENT_ASSERT(pe->in_use); diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 886bfb408..2d4d284fa 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -2572,9 +2572,6 @@ namespace libtorrent TORRENT_ASSERT(j->storage->num_outstanding_jobs() == 1); mutex::scoped_lock l(m_cache_mutex); -#if TORRENT_USE_ASSERTS - m_disk_cache.mark_deleted(*j->storage->files()); -#endif flush_cache(j->storage.get(), flush_delete_cache | flush_expect_clear , completed_jobs, l); From 2d1ce777f5576e2b382bf90aa7ef63f0151d4874 Mon Sep 17 00:00:00 2001 From: arvidn Date: Thu, 3 Nov 2016 17:06:53 -0400 Subject: [PATCH 2/2] fix python exception --- bindings/python/src/session.cpp | 2 +- bindings/python/test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/python/src/session.cpp b/bindings/python/src/session.cpp index 1bcafbe3d..cbf78323c 100644 --- a/bindings/python/src/session.cpp +++ b/bindings/python/src/session.cpp @@ -114,7 +114,7 @@ namespace int sett = setting_by_name(key); if (sett < 0) { - PyErr_SetString(PyExc_ValueError, ("unknown name in settings_pack: " + key).c_str()); + PyErr_SetString(PyExc_KeyError, ("unknown name in settings_pack: " + key).c_str()); throw_error_already_set(); } diff --git a/bindings/python/test.py b/bindings/python/test.py index 76fce40b5..c3115e74d 100644 --- a/bindings/python/test.py +++ b/bindings/python/test.py @@ -169,7 +169,7 @@ class test_session(unittest.TestCase): try: s = lt.session({'unexpected-key-name': 42}) self.assertFalse('should have thrown an exception') - except Exception as e: + except KeyError as e: print(e) def test_deprecated_settings(self):