more robust fix for connect spammage

panda/src/downloader/config_downloader.cxx

@@ -85,6 +85,11 @@ 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.
+const double connect_timeout =
+config_downloader.GetDouble("connect-timeout", 5.0);
+
 ConfigureFn(config_downloader) {
 #ifdef HAVE_SSL
   HTTPChannel::init_type();

panda/src/downloader/config_downloader.h

@@ -42,5 +42,6 @@ extern const bool early_random_seed;
 extern const bool verify_ssl;
 extern const string http_proxy;
 extern const string http_proxy_username;
+extern const double connect_timeout;
 
 #endif

panda/src/downloader/httpChannel.I

@@ -185,6 +185,65 @@ get_persistent_connection() const {
   return _persistent_connection;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::set_connect_timeout
+//       Access: Published
+//  Description: Sets the maximum length of time, in seconds, that the
+//               channel will wait before giving up on establishing a
+//               TCP connection.
+//
+//               At the 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_connect_timeout(double connect_timeout) {
+  _connect_timeout = connect_timeout;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::get_connect_timeout
+//       Access: Published
+//  Description: Returns the length of time, in seconds, to wait for a
+//               new nonblocking socket to connect.  See
+//               set_connect_timeout().
+////////////////////////////////////////////////////////////////////
+INLINE double HTTPChannel::
+get_connect_timeout() const {
+  return _connect_timeout;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::set_blocking_connect
+//       Access: Published
+//  Description: If this flag is true, a socket connect will block
+//               even for nonblocking I/O calls like
+//               begin_get_document(), begin_connect_to(), etc.  If
+//               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.).
+////////////////////////////////////////////////////////////////////
+INLINE void HTTPChannel::
+set_blocking_connect(bool blocking_connect) {
+  _blocking_connect = blocking_connect;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::get_blocking_connect
+//       Access: Published
+//  Description: If this flag is true, a socket connect will block
+//               even for nonblocking I/O calls like
+//               begin_get_document(), begin_connect_to(), etc.  If
+//               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.).
+////////////////////////////////////////////////////////////////////
+INLINE bool HTTPChannel::
+get_blocking_connect() const {
+  return _blocking_connect;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: HTTPChannel::set_download_throttle
 //       Access: Published

panda/src/downloader/httpChannel.cxx

@@ -50,6 +50,8 @@ HTTPChannel(HTTPClient *client) :
   _client(client)
 {
   _persistent_connection = false;
+  _connect_timeout = connect_timeout;
+  _blocking_connect = false;
   _download_throttle = false;
   _max_bytes_per_second = downloader_byte_rate;
   _seconds_per_update = downloader_frequency;
@@ -273,6 +275,8 @@ run() {
       }
       
       _state = S_connecting;
+      _started_connecting_time = 
+        ClockObject::get_global_clock()->get_real_time();
     }
 
     if (downloader_cat.is_spam()) {
@@ -285,6 +289,10 @@ run() {
       repeat_later = run_connecting();
       break;
       
+    case S_connecting_wait:
+      repeat_later = run_connecting_wait();
+      break;
+      
     case S_proxy_ready:
       repeat_later = run_proxy_ready();
       break;
@@ -593,23 +601,8 @@ run_connecting() {
   _status_string = string();
   if (BIO_do_connect(*_bio) <= 0) {
     if (BIO_should_retry(*_bio)) {
-
-      /* Put a block here for now. */
-      int fd = -1;
-      BIO_get_fd(*_bio, &fd);
-      if (fd < 0) {
-        downloader_cat.warning()
-          << "nonblocking socket BIO has no file descriptor.\n";
-      } else {
-        downloader_cat.spam()
-          << "waiting to connect.\n";
-        fd_set wset;
-        FD_ZERO(&wset);
-        FD_SET(fd, &wset);
-        select(fd + 1, NULL, &wset, NULL, NULL);
-      }        
-
-      return true;
+      _state = S_connecting_wait;
+      return false;
     }
     downloader_cat.info()
       << "Could not connect to " << _bio->get_server_name() << ":" 
@@ -617,7 +610,6 @@ run_connecting() {
 #ifdef REPORT_SSL_ERRORS
     ERR_print_errors_fp(stderr);
 #endif
-    free_bio();
     _state = S_failure;
     return false;
   }
@@ -637,6 +629,72 @@ run_connecting() {
   return false;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::run_connecting_wait
+//       Access: Private
+//  Description: Here we have begun to establish a nonblocking
+//               connection, but we got a come-back-later message, so
+//               we are waiting for the socket to finish connecting.
+////////////////////////////////////////////////////////////////////
+bool HTTPChannel::
+run_connecting_wait() {
+  int fd = -1;
+  BIO_get_fd(*_bio, &fd);
+  if (fd < 0) {
+    downloader_cat.warning()
+      << "nonblocking socket BIO has no file descriptor.\n";
+    _state = S_failure;
+    return false;
+  }
+
+  if (downloader_cat.is_debug()) {
+    downloader_cat.debug()
+      << "waiting to connect to " << _url.get_server() << ":" 
+      << _url.get_port() << ".\n";
+  }
+  fd_set wset;
+  FD_ZERO(&wset);
+  FD_SET(fd, &wset);
+  struct timeval tv;
+  if (get_blocking_connect()) {
+    // Since we'll be blocking on this connect, fill in the timeout
+    // into the structure.
+    tv.tv_sec = (int)_connect_timeout;
+    tv.tv_usec = (int)((_connect_timeout - tv.tv_sec) * 1000000.0);
+  } else {
+    // We won't block on this connect, so select() for 0 time.
+    tv.tv_sec = 0;
+    tv.tv_usec = 0;
+  }
+  int errcode = select(fd + 1, NULL, &wset, NULL, &tv);
+  if (errcode < 0) {
+    downloader_cat.warning()
+      << "Error in select.\n";
+    _state = S_failure;
+    return false;
+  }
+  
+  if (errcode == 0) {
+    // Nothing's happened so far; come back later.
+    if (get_blocking_connect() ||
+        (ClockObject::get_global_clock()->get_real_time() - 
+         _started_connecting_time > get_connect_timeout())) {
+      // Time to give up.
+      downloader_cat.info()
+        << "Timeout connecting to " << _url.get_server() << ":" 
+        << _url.get_port() << ".\n";
+      _state = S_failure;
+      return false;
+    }
+    return true;
+  }
+  
+  // The socket is now ready for writing.
+  _state = S_connecting;
+  return false;
+}
+
+
 ////////////////////////////////////////////////////////////////////
 //     Function: HTTPChannel::run_proxy_ready
 //       Access: Private

panda/src/downloader/httpChannel.h

@@ -87,6 +87,11 @@ PUBLISHED:
   INLINE void set_persistent_connection(bool persistent_connection);
   INLINE bool get_persistent_connection() const;
 
+  INLINE void set_connect_timeout(double timeout_seconds);
+  INLINE double get_connect_timeout() const;
+  INLINE void set_blocking_connect(bool blocking_connect);
+  INLINE bool get_blocking_connect() const;
+
   INLINE void set_download_throttle(bool download_throttle);
   INLINE bool get_download_throttle() const;
 
@@ -136,6 +141,7 @@ private:
 
   bool reached_done_state();
   bool run_connecting();
+  bool run_connecting_wait();
   bool run_proxy_ready();
   bool run_proxy_request_sent();
   bool run_proxy_reading_header();
@@ -196,6 +202,8 @@ private:
   PT(BioPtr) _bio;
   PT(BioStreamPtr) _source;
   bool _persistent_connection;
+  double _connect_timeout;
+  bool _blocking_connect;
   bool _download_throttle;
   double _max_bytes_per_second;
   double _max_updates_per_second;
@@ -256,6 +264,7 @@ private:
   enum State {
     S_new,
     S_connecting,
+    S_connecting_wait,
     S_proxy_ready,
     S_proxy_request_sent,
     S_proxy_reading_header,
@@ -273,6 +282,7 @@ private:
   };
   State _state;
   State _done_state;
+  double _started_connecting_time;
   bool _started_download;
   string _proxy_header;
   string _proxy_request_text;