I suggest we:

A) mark the time when the function is first entered.

B) when select() is about to be called, we can check how long time that
   has elapsed since the entry time and just do the math on how long the
   select timeout may be (this time).
Ok, that seems reasonable to me. I've attached a new patch that does this (I think), and still passes my test cases. I assumed that because the macro BLOCK_ADJUST (and BLOCK_ADJUST_ERRNO) were the only places we'd end up in _libssh2_wait_socket, that this was as good a place as any to mark the entry time, and they all seem to be at the beginning of exposed API functions.

Any further thoughts?

Matt


--

 / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel



--
_____________________________________________
Matt Lilley
Software Engineer
SecuritEase

Tel:    +64 4 912-2100
Fax:    +64 4 912-2101
E-mail: matt.lil...@securitease.com
Web:    http://www.securitease.com
_____________________________________________

This e-mail has passed our content security scan.
It is covered by the confidentiality clauses at 
http://www.securitease.com/content_and_confidentiality

>From dae57c891dd99cff7b8811b05a39593f9237ba55 Mon Sep 17 00:00:00 2001
From: Matt Lilley <matt.lil...@securitease.com>
Date: Tue, 3 May 2011 11:20:24 +1200
Subject: [PATCH] Fixes bug #160 as per Daniel's suggestion

---
 include/libssh2.h  |    4 ++++
 src/libssh2_priv.h |    3 +++
 src/session.c      |   51 +++++++++++++++++++++++++++++++++++++++++++++------
 src/session.h      |   19 ++++++++++++-------
 4 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/include/libssh2.h b/include/libssh2.h
index e207411..9462360 100644
--- a/include/libssh2.h
+++ b/include/libssh2.h
@@ -705,6 +705,10 @@ LIBSSH2_API int 
libssh2_session_get_blocking(LIBSSH2_SESSION* session);
 LIBSSH2_API void libssh2_channel_set_blocking(LIBSSH2_CHANNEL *channel,
                                               int blocking);
 
+LIBSSH2_API void libssh2_session_set_timeout(LIBSSH2_SESSION* session,
+                                             long timeout);
+LIBSSH2_API long libssh2_session_get_timeout(LIBSSH2_SESSION* session);
+
 /* libssh2_channel_handle_extended_data is DEPRECATED, do not use! */
 LIBSSH2_API void libssh2_channel_handle_extended_data(LIBSSH2_CHANNEL *channel,
                                                       int ignore_mode);
diff --git a/src/libssh2_priv.h b/src/libssh2_priv.h
index 231401e..5a697c7 100644
--- a/src/libssh2_priv.h
+++ b/src/libssh2_priv.h
@@ -559,6 +559,9 @@ struct _LIBSSH2_SESSION
     /* this is set to TRUE if a blocking API behavior is requested */
     int api_block_mode;
 
+    /* Timeout used when blocking API behavior is active */
+    long api_timeout;
+
     /* Server's public key */
     const LIBSSH2_HOSTKEY_METHOD *hostkey;
     void *server_hostkey_abstract;
diff --git a/src/session.c b/src/session.c
index e4714e8..edf98d8 100644
--- a/src/session.c
+++ b/src/session.c
@@ -481,6 +481,7 @@ libssh2_session_init_ex(LIBSSH2_ALLOC_FUNC((*my_alloc)),
         session->free = local_free;
         session->realloc = local_realloc;
         session->abstract = abstract;
+        session->api_timeout = 0; /* timeout-free API by default */
         session->api_block_mode = 1; /* blocking API by default */
         _libssh2_debug(session, LIBSSH2_TRACE_TRANS,
                        "New session resource allocated");
@@ -542,11 +543,12 @@ libssh2_session_callback_set(LIBSSH2_SESSION * session,
  * Utility function that waits for action on the socket. Returns 0 when ready
  * to run again or error on timeout.
  */
-int _libssh2_wait_socket(LIBSSH2_SESSION *session)
+int _libssh2_wait_socket(LIBSSH2_SESSION *session, time_t start_time)
 {
     int rc;
     int seconds_to_next;
     int dir;
+    int has_timeout;
 
     /* since libssh2 often sets EAGAIN internally before this function is
        called, we can decrease some amount of confusion in user programs by
@@ -592,8 +594,25 @@ int _libssh2_wait_socket(LIBSSH2_SESSION *session)
             fd_set *readfd = NULL;
             struct timeval tv;
 
-            tv.tv_sec = seconds_to_next;
-            tv.tv_usec = 0;
+            if (seconds_to_next > 0) {
+                tv.tv_sec = seconds_to_next;
+                tv.tv_usec = 0;
+                has_timeout = 1;
+            } else if (session->api_timeout > 0) {
+                time_t now = time (NULL);
+                long elapsed_ms = (long)(1000*difftime(start_time, now));      
          
+                if (elapsed_ms > session->api_timeout) {
+                    session->err_code = LIBSSH2_ERROR_TIMEOUT;
+                    return LIBSSH2_ERROR_TIMEOUT;
+                }
+                /* Seconds */
+                tv.tv_sec = (session->api_timeout - elapsed_ms) / 1000;
+                /* Microseconds */
+                tv.tv_usec = ((session->api_timeout - elapsed_ms) % 1000) * 
1000;
+                has_timeout = 1;
+            } else {
+                has_timeout = 0;
+            }
 
             if(dir & LIBSSH2_SESSION_BLOCK_INBOUND) {
                 FD_ZERO(&rfd);
@@ -607,10 +626,8 @@ int _libssh2_wait_socket(LIBSSH2_SESSION *session)
                 writefd = &wfd;
             }
 
-            /* Note that this COULD be made to use a timeout that perhaps
-               could be customizable by the app or something... */
             rc = select(session->socket_fd + 1, readfd, writefd, NULL,
-                        seconds_to_next ? &tv : NULL);
+                        has_timeout ? &tv : NULL);
 #endif
         }
     }
@@ -1281,6 +1298,28 @@ libssh2_session_get_blocking(LIBSSH2_SESSION * session)
     return session->api_block_mode;
 }
 
+
+/* libssh2_session_set_timeout
+ *
+ * Set a session's timeout (in msec) for blocking mode, 
+ * or 0 to disable timeouts.
+ */
+LIBSSH2_API void
+libssh2_session_set_timeout(LIBSSH2_SESSION * session, long timeout)
+{
+    session->api_timeout = timeout;
+}
+
+/* libssh2_session_get_timeout
+ *
+ * Returns a session's timeout, or 0 if disabled
+ */
+LIBSSH2_API long
+libssh2_session_get_timeout(LIBSSH2_SESSION * session)
+{
+    return session->api_timeout;
+}
+
 /*
  * libssh2_poll_channel_read
  *
diff --git a/src/session.h b/src/session.h
index 4eac3a6..c7e01e9 100644
--- a/src/session.h
+++ b/src/session.h
@@ -51,17 +51,20 @@
    function.
 
 */
-#define BLOCK_ADJUST(rc,sess,x) \
+#define BLOCK_ADJUST(rc,sess,x)          { \
+    time_t entry_time = time (NULL); \
     do { \
        rc = x; \
        /* the order of the check below is important to properly deal with the
           case when the 'sess' is freed */ \
        if((rc != LIBSSH2_ERROR_EAGAIN) || !sess->api_block_mode)  \
            break; \
-       rc = _libssh2_wait_socket(sess); \
+       rc = _libssh2_wait_socket(sess, entry_time);     \
        if(rc) \
            break; \
-    } while(1)
+    } while(1); \
+}
+
 
 /*
  * For functions that returns a pointer, we need to check if the API is
@@ -69,7 +72,8 @@
  * immediately. If the API is blocking and we get a NULL we check the errno
  * and *only* if that is EAGAIN we loop and wait for socket action.
  */
-#define BLOCK_ADJUST_ERRNO(ptr,sess,x)          \
+#define BLOCK_ADJUST_ERRNO(ptr,sess,x)          { \
+    time_t entry_time = time (NULL); \
     do { \
        int rc; \
        ptr = x; \
@@ -77,13 +81,14 @@
           (ptr != NULL) || \
           (libssh2_session_last_errno(sess) != LIBSSH2_ERROR_EAGAIN) ) \
            break;                                                  \
-       rc = _libssh2_wait_socket(sess); \
+       rc = _libssh2_wait_socket(sess, entry_time);                           \
        if(rc) \
            break; \
-    } while(1)
+    } while(1);   \
+}
 
 
-int _libssh2_wait_socket(LIBSSH2_SESSION *session);
+int _libssh2_wait_socket(LIBSSH2_SESSION *session, time_t entry_time);
 
 /* this is the lib-internal set blocking function */
 int _libssh2_session_set_blocking(LIBSSH2_SESSION * session, int blocking);
-- 
1.7.2.3.msysgit.0

_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel

Reply via email to