From 7dd11268ddc934bc4fe5906738747153286a9cc6 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 23 Apr 2017 11:15:43 -0400 Subject: [PATCH 1/5] removed use of boost::uintptr_t for better compatibility --- ChangeLog | 1 + src/lsd.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 5689160aa..4fe8af572 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * removed depdendency on boost::uintptr_t for better compatibility * fix memory leak in the disk cache * fix double free in disk cache * forward declaring libtorrent types is discouraged. a new fwd.hpp header is provided diff --git a/src/lsd.cpp b/src/lsd.cpp index a46e2a471..ff8acf158 100644 --- a/src/lsd.cpp +++ b/src/lsd.cpp @@ -87,7 +87,7 @@ lsd::lsd(io_service& ios, peer_callback_t const& cb , m_log_cb(log) #endif , m_broadcast_timer(ios) - , m_cookie((random() ^ boost::uintptr_t(this)) & 0x7fffffff) + , m_cookie((random() ^ uintptr_t(this)) & 0x7fffffff) , m_disabled(false) #if TORRENT_USE_IPV6 , m_disabled6(false) From afe1f685a4117766990ed4bbb4d8c63e3c1df7b3 Mon Sep 17 00:00:00 2001 From: Jan Berkel Date: Sun, 23 Apr 2017 18:56:31 +0200 Subject: [PATCH 2/5] Set next/min_announce to now Prevents unnecessary tracker announce delays #1940 --- src/torrent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/torrent.cpp b/src/torrent.cpp index ded965319..18e9726a6 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -3308,8 +3308,8 @@ namespace libtorrent } ae.updating = true; - ae.next_announce = now + seconds(20); - ae.min_announce = now + seconds(10); + ae.next_announce = now; + ae.min_announce = now; if (m_ses.alerts().should_post()) { From 1ea760ae93caf3a580edc3960d30afaf3c12f58c Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 24 Apr 2017 00:44:04 -0400 Subject: [PATCH 3/5] fix iconv cast warnings --- include/libtorrent/config.hpp | 6 +++--- src/escape_string.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp index 33b9657f4..be22e9c3f 100644 --- a/include/libtorrent/config.hpp +++ b/include/libtorrent/config.hpp @@ -199,7 +199,7 @@ POSSIBILITY OF SUCH DAMAGE. // FreeBSD has a reasonable iconv signature // unless we're on glibc #ifndef __GLIBC__ -# define TORRENT_ICONV_ARG (const char**) +# define TORRENT_ICONV_ARG(x) (x) #endif #endif // __APPLE__ @@ -344,7 +344,7 @@ POSSIBILITY OF SUCH DAMAGE. #define TORRENT_USE_IFCONF 1 #define TORRENT_USE_SYSCTL 1 #define TORRENT_USE_IPV6 0 -#define TORRENT_ICONV_ARG (const char**) +#define TORRENT_ICONV_ARG(x) (x) #define TORRENT_USE_WRITEV 0 #define TORRENT_USE_READV 0 @@ -457,7 +457,7 @@ int snprintf(char* buf, int len, char const* fmt, ...) #endif #ifndef TORRENT_ICONV_ARG -#define TORRENT_ICONV_ARG (char**) +#define TORRENT_ICONV_ARG(x) const_cast(x) #endif #if defined __GNUC__ || defined __clang__ diff --git a/src/escape_string.cpp b/src/escape_string.cpp index c910d446c..2b892e059 100644 --- a/src/escape_string.cpp +++ b/src/escape_string.cpp @@ -514,6 +514,7 @@ namespace libtorrent #endif #if TORRENT_USE_ICONV +namespace { std::string iconv_convert_impl(std::string const& s, iconv_t h) { std::string ret; @@ -525,9 +526,9 @@ namespace libtorrent // posix has a weird iconv signature. implementations // differ on what this signature should be, so we use // a macro to let config.hpp determine it - size_t retval = iconv(h, TORRENT_ICONV_ARG &in, &insize, + size_t retval = iconv(h, TORRENT_ICONV_ARG(&in), &insize, &out, &outsize); - if (retval == (size_t)-1) return s; + if (retval == size_t(-1)) return s; // if this string has an invalid utf-8 sequence in it, don't touch it if (insize != 0) return s; // not sure why this would happen, but it seems to be possible @@ -537,6 +538,7 @@ namespace libtorrent ret.resize(ret.size() - outsize); return ret; } +} // anonymous namespace std::string convert_to_native(std::string const& s) { From fe9f877087c66c746f36eaf1547bd8872a256b55 Mon Sep 17 00:00:00 2001 From: Jan Berkel Date: Thu, 4 May 2017 23:32:47 +0200 Subject: [PATCH 4/5] Set connection timeout when next endpoint is tried (#1952) --- ChangeLog | 1 + simulation/libsimulator | 2 +- simulation/test_http_connection.cpp | 109 +++++++++++++++++++++++++++- src/http_connection.cpp | 4 +- 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4fe8af572..12ac76686 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * fix http connection timeout on multi-homed hosts * removed depdendency on boost::uintptr_t for better compatibility * fix memory leak in the disk cache * fix double free in disk cache diff --git a/simulation/libsimulator b/simulation/libsimulator index 60d786b8f..0e8d74baf 160000 --- a/simulation/libsimulator +++ b/simulation/libsimulator @@ -1 +1 @@ -Subproject commit 60d786b8fa6ddaacdc98bdf691220660bc194494 +Subproject commit 0e8d74baf1f6d9db19857eaa87734faecd530f94 diff --git a/simulation/test_http_connection.cpp b/simulation/test_http_connection.cpp index c9d059a65..74de4c777 100644 --- a/simulation/test_http_connection.cpp +++ b/simulation/test_http_connection.cpp @@ -83,6 +83,13 @@ struct sim_config : sim::default_config return duration_cast(chrono::milliseconds(100)); } + if (hostname == "dual-stack.test-hostname.com") + { + result.push_back(address_v4::from_string("10.0.0.2")); + result.push_back(address_v6::from_string("ff::dead:beef")); + return duration_cast(chrono::milliseconds(100)); + } + return default_config::hostname_lookup(requestor, hostname, result, ec); } }; @@ -225,8 +232,12 @@ void run_suite(lt::aux::proxy_settings ps) if (ps.type != settings_pack::socks5 && ps.type != settings_pack::http) { + const auto expected_code = ps.type == settings_pack::socks4 ? + boost::system::errc::address_family_not_supported : + boost::system::errc::address_not_available; + run_test(ps, "http://[ff::dead:beef]:8080/test_file", 0, -1 - , error_condition(boost::system::errc::address_family_not_supported, generic_category()) + , error_condition(expected_code, generic_category()) , {0,1}); } @@ -438,6 +449,102 @@ TORRENT_TEST(http_connection_socks5_proxy_names) run_suite(ps); } +// tests the error scenario of a http server listening on two sockets (ipv4/ipv6) which +// both accept the incoming connection but never send anything back. we test that +// both ip addresses get tried in turn and that the connection attempts time out as expected. +TORRENT_TEST(http_connection_timeout_server_stalls) +{ + sim_config network_cfg; + sim::simulation sim{network_cfg}; + // server has two ip addresses (ipv4/ipv6) + sim::asio::io_service server_ios(sim, address_v4::from_string("10.0.0.2")); + sim::asio::io_service server_ios_ipv6(sim, address_v6::from_string("ff::dead:beef")); + // same for client + sim::asio::io_service client_ios(sim, { + address_v4::from_string("10.0.0.1"), + address_v6::from_string("ff::abad:cafe") + }); + lt::resolver resolver(client_ios); + + const unsigned short http_port = 8080; + sim::http_server http(server_ios, http_port); + sim::http_server http_ipv6(server_ios_ipv6, http_port); + + http.register_stall_handler("/timeout"); + http_ipv6.register_stall_handler("/timeout"); + + char data_buffer[4000]; + std::generate(data_buffer, data_buffer + sizeof(data_buffer), &std::rand); + + int connect_counter = 0; + int handler_counter = 0; + + error_condition timed_out(boost::system::errc::timed_out, boost::system::generic_category()); + + auto c = test_request(client_ios, resolver + , "http://dual-stack.test-hostname.com:8080/timeout", data_buffer, -1, -1 + , timed_out, lt::aux::proxy_settings() + , &connect_counter, &handler_counter); + + error_code e; + sim.run(e); + TEST_CHECK(!e); + TEST_EQUAL(2, connect_counter); // both endpoints are connected to + TEST_EQUAL(1, handler_counter); // the handler only gets called once with error_code == timed_out +} + +// tests the error scenario of a http server listening on two sockets (ipv4/ipv6) neither of which +// accept incoming connections. we test that both ip addresses get tried in turn and that the +// connection attempts time out as expected. +TORRENT_TEST(http_connection_timeout_server_does_not_accept) +{ + sim_config network_cfg; + sim::simulation sim{network_cfg}; + // server has two ip addresses (ipv4/ipv6) + sim::asio::io_service server_ios(sim, { + address_v4::from_string("10.0.0.2"), + address_v6::from_string("ff::dead:beef") + }); + // same for client + sim::asio::io_service client_ios(sim, { + address_v4::from_string("10.0.0.1"), + address_v6::from_string("ff::abad:cafe") + }); + lt::resolver resolver(client_ios); + + const unsigned short http_port = 8080; + + // listen on two sockets, but don't accept connections + asio::ip::tcp::acceptor server_socket_ipv4(server_ios); + server_socket_ipv4.open(tcp::v4()); + server_socket_ipv4.bind(tcp::endpoint(address_v4::any(), http_port)); + server_socket_ipv4.listen(); + + asio::ip::tcp::acceptor server_socket_ipv6(server_ios); + server_socket_ipv6.open(tcp::v6()); + server_socket_ipv6.bind(tcp::endpoint(address_v6::any(), http_port)); + server_socket_ipv6.listen(); + + int connect_counter = 0; + int handler_counter = 0; + + error_condition timed_out(boost::system::errc::timed_out, boost::system::generic_category()); + + char data_buffer[4000]; + std::generate(data_buffer, data_buffer + sizeof(data_buffer), &std::rand); + + auto c = test_request(client_ios, resolver + , "http://dual-stack.test-hostname.com:8080/timeout_server_does_not_accept", data_buffer, -1, -1 + , timed_out, lt::aux::proxy_settings() + , &connect_counter, &handler_counter); + + error_code e; + sim.run(e); + TEST_CHECK(!e); + TEST_EQUAL(0, connect_counter); // no connection takes place + TEST_EQUAL(1, handler_counter); // the handler only gets called once with error_code == timed_out +} + void test_proxy_failure(lt::settings_pack::proxy_type_t proxy_type) { using sim::asio::ip::address_v4; diff --git a/src/http_connection.cpp b/src/http_connection.cpp index 37b28aa38..f599e8356 100644 --- a/src/http_connection.cpp +++ b/src/http_connection.cpp @@ -449,12 +449,14 @@ void http_connection::on_timeout(boost::weak_ptr p error_code ec; c->m_sock.close(ec); if (!c->m_connecting) c->connect(); + c->m_last_receive = now; + c->m_start_time = c->m_last_receive; } else { c->callback(boost::asio::error::timed_out); + return; } - return; } else { From 14dbd1c92dfea5bb3b2d713d27170e1270a7c779 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 7 May 2017 11:01:42 -0400 Subject: [PATCH 5/5] fix race condition in disk I/O storage class --- ChangeLog | 1 + include/libtorrent/storage.hpp | 1 + src/storage.cpp | 9 +++++++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 12ac76686..ba72f7f9f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * fix race condition in disk I/O storage class * fix http connection timeout on multi-homed hosts * removed depdendency on boost::uintptr_t for better compatibility * fix memory leak in the disk cache diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index da894d7f9..844071d28 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -476,6 +476,7 @@ namespace libtorrent // whose bit is 0, we set the file size, to make the file allocated // on disk (in full allocation mode) and just sparsely allocated in // case of sparse allocation mode + mutable mutex m_file_created_mutex; mutable bitfield m_file_created; bool m_allocate_files; diff --git a/src/storage.cpp b/src/storage.cpp index a22c70e5f..9c0b7b1ce 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -524,7 +524,10 @@ namespace libtorrent m_allocate_files = false; #endif - m_file_created.resize(files().num_files(), false); + { + mutex::scoped_lock l(m_file_created_mutex); + m_file_created.resize(files().num_files(), false); + } // first, create all missing directories std::string last_path; @@ -1494,6 +1497,7 @@ namespace libtorrent if (m_allocate_files && (mode & file::rw_mask) != file::read_only) { + mutex::scoped_lock l(m_file_created_mutex); if (m_file_created.size() != files().num_files()) m_file_created.resize(files().num_files(), false); @@ -1504,10 +1508,11 @@ namespace libtorrent // the file right away, to allocate it on the filesystem. if (m_file_created[file] == false) { + m_file_created.set_bit(file); + l.unlock(); error_code e; boost::int64_t const size = files().file_size(file); h->set_size(size, e); - m_file_created.set_bit(file); if (e) { ec.ec = e;