diff --git a/ChangeLog b/ChangeLog index bcfb3f7f6..8db6ceb19 100644 --- a/ChangeLog +++ b/ChangeLog @@ -73,6 +73,9 @@ * require C++11 to build libtorrent + * 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 * fix double free in disk cache * forward declaring libtorrent types is discouraged. a new fwd.hpp header is provided diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp index bd459ea73..80cbca3ba 100644 --- a/include/libtorrent/config.hpp +++ b/include/libtorrent/config.hpp @@ -177,7 +177,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__ @@ -333,7 +333,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 @@ -361,7 +361,7 @@ POSSIBILITY OF SUCH DAMAGE. #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/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index 21b8649de..b5c26fb67 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -442,6 +442,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 std::mutex m_file_created_mutex; mutable typed_bitfield m_file_created; bool m_allocate_files; 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 ae7eb8ea8..f301448cb 100644 --- a/simulation/test_http_connection.cpp +++ b/simulation/test_http_connection.cpp @@ -86,6 +86,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); } }; @@ -171,7 +178,7 @@ std::shared_ptr test_request(io_service& ios std::printf("CONNECTED: %s\n", url.c_str()); }); - h->get(url, seconds(1), 0, &ps, 5, "test/user-agent", address(address_v4::any()) + h->get(url, seconds(1), 0, &ps, 5, "test/user-agent", boost::optional
() , 0, auth); return h; } @@ -228,8 +235,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}); } @@ -441,6 +452,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(connect_counter, 2); // both endpoints are connected to + TEST_EQUAL(handler_counter, 1); // 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(connect_counter, 0); // no connection takes place + TEST_EQUAL(handler_counter, 1); // 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/escape_string.cpp b/src/escape_string.cpp index c86525d61..62a973b13 100644 --- a/src/escape_string.cpp +++ b/src/escape_string.cpp @@ -548,6 +548,7 @@ namespace libtorrent { #endif #if TORRENT_USE_ICONV +namespace { std::string iconv_convert_impl(std::string const& s, iconv_t h) { std::string ret; @@ -559,9 +560,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 @@ -571,6 +572,7 @@ namespace libtorrent { ret.resize(ret.size() - outsize); return ret; } +} // anonymous namespace std::string convert_to_native(std::string const& s) { diff --git a/src/http_connection.cpp b/src/http_connection.cpp index 2fa889f27..77723c042 100644 --- a/src/http_connection.cpp +++ b/src/http_connection.cpp @@ -435,12 +435,14 @@ void http_connection::on_timeout(std::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 { diff --git a/src/storage.cpp b/src/storage.cpp index 9e407420b..7ca60197e 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -218,7 +218,10 @@ namespace libtorrent { m_allocate_files = false; #endif - m_file_created.resize(files().num_files(), false); + { + std::unique_lock l(m_file_created_mutex); + m_file_created.resize(files().num_files(), false); + } // first, create all missing directories std::string last_path; @@ -630,6 +633,7 @@ namespace libtorrent { if (m_allocate_files && (mode & file::rw_mask) != file::read_only) { + std::unique_lock l(m_file_created_mutex); if (m_file_created.size() != files().num_files()) m_file_created.resize(files().num_files(), false); @@ -640,10 +644,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; std::int64_t const size = files().file_size(file); h->set_size(size, e); - m_file_created.set_bit(file); if (e) { ec.ec = e; diff --git a/src/torrent.cpp b/src/torrent.cpp index 96bb13a1c..7a29f23e6 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -2851,8 +2851,8 @@ namespace libtorrent { } aep.updating = true; - aep.next_announce = now + seconds32(20); - aep.min_announce = now + seconds32(10); + aep.next_announce = now; + aep.min_announce = now; if (m_ses.alerts().should_post()) {