diff --git a/panda/src/downloader/bioStreamBuf.cxx b/panda/src/downloader/bioStreamBuf.cxx index c2e1c05008..773048caf7 100644 --- a/panda/src/downloader/bioStreamBuf.cxx +++ b/panda/src/downloader/bioStreamBuf.cxx @@ -165,11 +165,18 @@ underflow() { if (read_count <= 0) { _is_closed = !BIO_should_retry(*_source); if (_is_closed) { - downloader_cat.info() - << "Socket error detected on " - << _source->get_server_name() << ":" - << _source->get_port() << ", return code " - << read_count << ".\n"; + if (read_count == 0) { + downloader_cat.info() + << "Lost connection to " + << _source->get_server_name() << ":" + << _source->get_port() << ".\n"; + } else { + downloader_cat.info() + << "Socket error detected on " + << _source->get_server_name() << ":" + << _source->get_port() << ", return code " + << read_count << ".\n"; + } #ifdef REPORT_OPENSSL_ERRORS ERR_print_errors_fp(stderr); #endif diff --git a/panda/src/downloader/config_downloader.cxx b/panda/src/downloader/config_downloader.cxx index 3981ec3bac..c67a73d4fb 100644 --- a/panda/src/downloader/config_downloader.cxx +++ b/panda/src/downloader/config_downloader.cxx @@ -85,11 +85,26 @@ config_downloader.GetString("http-proxy", ""); const string http_proxy_username = config_downloader.GetString("http-proxy-username", ""); -// This is the default amount of time to wait for a connection, in -// seconds. It is presently only used for nonblocking sockets. +// This is the default amount of time to wait for a TCP/IP connection +// to be established, in seconds. It is presently only used for +// nonblocking sockets. const double connect_timeout = config_downloader.GetDouble("connect-timeout", 5.0); +// This is the default amount of time to wait for the HTTP server to +// finish sending its response to our request, in seconds. It starts +// counting after the TCP connection has been established +// (connect_timeout, above) and the request has been sent. +const double http_timeout = +config_downloader.GetDouble("http-timeout", 20.0); + +// This is the maximum number of times to try reconnecting to the +// server on any one document attempt. This is just a failsafe to +// prevent the code from attempting runaway connections; this limit +// should never be reached in practice. +const int http_max_connect_count = +config_downloader.GetInt("http-max-connect-count", 10); + ConfigureFn(config_downloader) { #ifdef HAVE_SSL HTTPChannel::init_type(); diff --git a/panda/src/downloader/config_downloader.h b/panda/src/downloader/config_downloader.h index 04d9cda0fe..177fd5cb98 100644 --- a/panda/src/downloader/config_downloader.h +++ b/panda/src/downloader/config_downloader.h @@ -45,6 +45,8 @@ extern const bool verify_ssl; extern const string http_proxy; extern const string http_proxy_username; extern const double connect_timeout; +extern const double http_timeout; +extern const int http_max_connect_count; // Later, we can make this conditional on NDEBUG or something along // those lines; for now, we define it always to be true so we get diff --git a/panda/src/downloader/httpChannel.I b/panda/src/downloader/httpChannel.I index 0da48431c3..305c6b3fc0 100644 --- a/panda/src/downloader/httpChannel.I +++ b/panda/src/downloader/httpChannel.I @@ -200,7 +200,7 @@ get_persistent_connection() const { // channel will wait before giving up on establishing a // TCP connection. // -// At the present, this is used only for the nonblocking +// At present, this is used only for the nonblocking // interfaces (e.g. begin_get_document(), // begin_connect_to()), but it is used whether // set_blocking_connect() is true or false. @@ -231,6 +231,14 @@ get_connect_timeout() const { // false, a socket connect will not block for // nonblocking I/O calls, but will block for blocking // I/O calls (get_document(), connect_to(), etc.). + +// Setting this true is useful when you want to use +// non-blocking I/O once you have established the +// connection, but you don't want to bother with polling +// for the initial connection. It's also useful when +// you don't particularly care about non-blocking I/O, +// but you need to respect timeouts like connect_timeout +// and http_timeout. //////////////////////////////////////////////////////////////////// INLINE void HTTPChannel:: set_blocking_connect(bool blocking_connect) { @@ -252,6 +260,39 @@ get_blocking_connect() const { return _blocking_connect; } +//////////////////////////////////////////////////////////////////// +// Function: HTTPChannel::set_http_timeout +// Access: Published +// Description: Sets the maximum length of time, in seconds, that the +// channel will wait for the HTTP server to finish +// sending its response to our request. +// +// The timer starts counting after the TCP connection +// has been established (see set_connect_timeout(), +// above) and the request has been sent. +// +// At present, this is used only for the nonblocking +// interfaces (e.g. begin_get_document(), +// begin_connect_to()), but it is used whether +// set_blocking_connect() is true or false. +//////////////////////////////////////////////////////////////////// +INLINE void HTTPChannel:: +set_http_timeout(double http_timeout) { + _http_timeout = http_timeout; +} + +//////////////////////////////////////////////////////////////////// +// Function: HTTPChannel::get_http_timeout +// Access: Published +// Description: Returns the length of time, in seconds, to wait for +// the HTTP server to respond to our request. See +// set_http_timeout(). +//////////////////////////////////////////////////////////////////// +INLINE double HTTPChannel:: +get_http_timeout() const { + return _http_timeout; +} + //////////////////////////////////////////////////////////////////// // Function: HTTPChannel::set_download_throttle // Access: Published diff --git a/panda/src/downloader/httpChannel.cxx b/panda/src/downloader/httpChannel.cxx index 44db8550e9..ae4f360ce8 100644 --- a/panda/src/downloader/httpChannel.cxx +++ b/panda/src/downloader/httpChannel.cxx @@ -43,6 +43,7 @@ HTTPChannel(HTTPClient *client) : { _persistent_connection = false; _connect_timeout = connect_timeout; + _http_timeout = http_timeout; _blocking_connect = false; _download_throttle = false; _max_bytes_per_second = downloader_byte_rate; @@ -254,6 +255,17 @@ run() { bool repeat_later; do { if (_bio.is_null()) { + if (_connect_count > http_max_connect_count) { + // Too many connection attempts, just give up. We should + // never trigger this failsafe, since the code in each + // individual case has similar logic to prevent more than two + // consecutive lost connections. + downloader_cat.warning() + << "Too many lost connections, giving up.\n"; + _state = S_failure; + return false; + } + // No connection. Attempt to establish one. _proxy = _client->get_proxy(); @@ -266,10 +278,17 @@ run() { if (_nonblocking) { BIO_set_nbio(*_bio, 1); } + + if (_connect_count > 0) { + downloader_cat.info() + << "Reconnecting to " << _bio->get_server_name() << ":" + << _bio->get_port() << "\n"; + } _state = S_connecting; _started_connecting_time = ClockObject::get_global_clock()->get_real_time(); + _connect_count++; } if (downloader_cat.is_spam()) { @@ -667,8 +686,7 @@ run_connecting_wait() { if (downloader_cat.is_debug()) { downloader_cat.debug() - << "waiting to connect to " << _url.get_server() << ":" - << _url.get_port() << ".\n"; + << "waiting to connect to " << _url.get_server_and_port() << ".\n"; } fd_set wset; FD_ZERO(&wset); @@ -699,8 +717,7 @@ run_connecting_wait() { _started_connecting_time > get_connect_timeout())) { // Time to give up. downloader_cat.info() - << "Timeout connecting to " << _url.get_server() << ":" - << _url.get_port() << ".\n"; + << "Timeout connecting to " << _url.get_server_and_port() << ".\n"; _state = S_failure; return false; } @@ -877,7 +894,7 @@ run_ssl_handshake() { } downloader_cat.info() << "Could not establish SSL handshake with " - << _url.get_server() << ":" << _url.get_port() << "\n"; + << _url.get_server_and_port() << "\n"; #ifdef REPORT_OPENSSL_ERRORS ERR_print_errors_fp(stderr); #endif @@ -904,8 +921,7 @@ run_ssl_handshake() { long verify_result = SSL_get_verify_result(ssl); if (verify_result == X509_V_ERR_CERT_HAS_EXPIRED) { downloader_cat.info() - << "Expired certificate from " << _url.get_server() << ":" - << _url.get_port() << "\n"; + << "Expired certificate from " << _url.get_server_and_port() << "\n"; if (_client->get_verify_ssl() == HTTPClient::VS_normal) { _state = S_failure; return false; @@ -913,8 +929,7 @@ run_ssl_handshake() { } else if (verify_result == X509_V_ERR_CERT_NOT_YET_VALID) { downloader_cat.info() - << "Premature certificate from " << _url.get_server() << ":" - << _url.get_port() << "\n"; + << "Premature certificate from " << _url.get_server_and_port() << "\n"; if (_client->get_verify_ssl() == HTTPClient::VS_normal) { _state = S_failure; return false; @@ -922,8 +937,8 @@ run_ssl_handshake() { } else if (verify_result != X509_V_OK) { downloader_cat.info() - << "Unable to verify identity of " << _url.get_server() << ":" - << _url.get_port() << ", verify error code " << verify_result << "\n"; + << "Unable to verify identity of " << _url.get_server_and_port() + << ", verify error code " << verify_result << "\n"; if (_client->get_verify_ssl() != HTTPClient::VS_no_verify) { _state = S_failure; return false; @@ -997,6 +1012,7 @@ run_ready() { // All done sending request. _state = S_request_sent; + _sent_request_time = ClockObject::get_global_clock()->get_real_time(); return false; } @@ -1021,18 +1037,23 @@ run_request_sent() { // Try again, once. _response_type = RT_hangup; } + + } else if (ClockObject::get_global_clock()->get_real_time() - + _sent_request_time > get_http_timeout()) { + // Time to give up. + downloader_cat.info() + << "Timeout waiting for " << _url.get_server_and_port() << ".\n"; + _state = S_failure; } + return true; } if (!parse_http_response(line)) { + // Not an HTTP response. _state is already set appropriately. return false; } - // Ok, we've established an HTTP connection to the server. Our - // extra send headers have done their job; clear them for next time. - clear_extra_headers(); - _state = S_reading_header; _current_field_name = string(); _current_field_value = string(); @@ -1052,8 +1073,32 @@ run_request_sent() { bool HTTPChannel:: run_reading_header() { if (parse_http_header()) { + if (_bio.is_null()) { + downloader_cat.info() + << "Connection lost while reading HTTP response.\n"; + if (_response_type == RT_http_hangup) { + // This was our second hangup in a row. Give up. + _state = S_failure; + + } else { + // Try again, once. + _response_type = RT_http_hangup; + } + + } else if (ClockObject::get_global_clock()->get_real_time() - + _sent_request_time > get_http_timeout()) { + // Time to give up. + downloader_cat.info() + << "Timeout waiting for " << _url.get_server_and_port() << ".\n"; + _state = S_failure; + } return true; } + _response_type = RT_http_complete; + + // Ok, we've established an HTTP connection to the server. Our + // extra send headers have done their job; clear them for next time. + clear_extra_headers(); _server_response_has_no_body = (get_status_code() / 100 == 1 || @@ -1490,6 +1535,7 @@ begin_request(HTTPEnum::Method method, const URLSpec &url, _first_byte = first_byte; _last_byte = last_byte; + _connect_count = 0; make_header(); make_request_text(); @@ -1752,9 +1798,6 @@ parse_http_response(const string &line) { return false; } - // Okay, we're sane. - _response_type = RT_http; - // Split out the first line into its three components. size_t p = 5; while (p < line.length() && !isspace(line[p])) { diff --git a/panda/src/downloader/httpChannel.h b/panda/src/downloader/httpChannel.h index 34adde59dd..1d9a3711d2 100644 --- a/panda/src/downloader/httpChannel.h +++ b/panda/src/downloader/httpChannel.h @@ -94,6 +94,9 @@ PUBLISHED: INLINE void set_blocking_connect(bool blocking_connect); INLINE bool get_blocking_connect() const; + INLINE void set_http_timeout(double timeout_seconds); + INLINE double get_http_timeout() const; + INLINE void set_download_throttle(bool download_throttle); INLINE bool get_download_throttle() const; @@ -204,6 +207,7 @@ private: PT(BioStreamPtr) _source; bool _persistent_connection; double _connect_timeout; + double _http_timeout; bool _blocking_connect; bool _download_throttle; double _max_bytes_per_second; @@ -223,6 +227,7 @@ private: bool _server_response_has_no_body; size_t _first_byte; size_t _last_byte; + int _connect_count; enum DownloadDest { DD_none, @@ -251,11 +256,13 @@ private: string _www_username; PT(HTTPAuthorization) _www_auth; + // What type of response do we get to our HTTP request? enum ResponseType { RT_none, - RT_hangup, - RT_non_http, - RT_http + RT_hangup, // immediately lost connection + RT_non_http, // something that wasn't an expected HTTP response + RT_http_hangup, // the start of an HTTP response, then a lost connection + RT_http_complete // a valid HTTP response completed }; ResponseType _response_type; @@ -295,6 +302,7 @@ private: State _state; State _done_state; double _started_connecting_time; + double _sent_request_time; bool _started_download; string _proxy_header; string _proxy_request_text;