Re: [libmicrohttpd] [PATCH] Add support for using a monotonic clock for timeouts

[Christian Grothoff]

Dear Mike,

Thanks for your patches.  I've committed all 3 of them in SVN 22116
(first two) and SVN 22117 (this one) respectively, so they will be part
of the next release.


Happy hacking!

Christian

On 06/19/2012 06:50 PM, Mike Crowe wrote:
> Change various uses of time(NULL) to new MHD_monotonic_time() function to
> make timeouts immune to the system real time clock changing. Leave one call
> to time(2) which continues to use the real time for the HTTP Date: header.
> 
> If clock_gettime does not support CLOCK_MONOTONIC then fall through to
> calling time(NULL) instead. This will be problematic if clock_gettime
> sometimes fails and sometimes succeeds but that shouldn't happen.
> 
> The autoconf magic was lifted from
> http://lists.gnu.org/archive/html/autoconf/2010-08/msg00035.html
> ---
>  configure.ac                  |    3 +++
>  src/daemon/connection.c       |    6 +++---
>  src/daemon/connection_https.c |    4 ++--
>  src/daemon/daemon.c           |    7 +++----
>  src/daemon/digestauth.c       |    4 ++--
>  src/daemon/internal.c         |   10 ++++++++++
>  src/daemon/internal.h         |    6 ++++++
>  7 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 41d5472..7b47278 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -190,6 +190,9 @@ 
> AC_CHECK_HEADERS([plibc.h],our_private_plibc_h=0,our_private_plibc_h=1)
>  AM_CONDITIONAL(USE_PRIVATE_PLIBC_H, test x$our_private_plibc_h = x1)    
>  
>  AC_CHECK_FUNCS(memmem)
> +AC_SEARCH_LIBS([clock_gettime], [rt], [
> +                             AC_DEFINE(HAVE_CLOCK_GETTIME, 1, [Have 
> clock_gettime])
> +])
>  
>  # IPv6
>  AC_MSG_CHECKING(for IPv6)
> diff --git a/src/daemon/connection.c b/src/daemon/connection.c
> index acc4839..13c465b 100644
> --- a/src/daemon/connection.c
> +++ b/src/daemon/connection.c
> @@ -1786,7 +1786,7 @@ parse_connection_headers (struct MHD_Connection 
> *connection)
>  int
>  MHD_connection_handle_read (struct MHD_Connection *connection)
>  {
> -  connection->last_activity = time (NULL);
> +  connection->last_activity = MHD_monotonic_time();
>    if (connection->state == MHD_CONNECTION_CLOSED)
>      return MHD_YES;
>    /* make sure "read" has a reasonable number of bytes
> @@ -1851,7 +1851,7 @@ MHD_connection_handle_write (struct MHD_Connection 
> *connection)
>  {
>    struct MHD_Response *response;
>    int ret;
> -  connection->last_activity = time (NULL);
> +  connection->last_activity = MHD_monotonic_time();
>    while (1)
>      {
>  #if DEBUG_STATES
> @@ -2372,7 +2372,7 @@ MHD_connection_handle_idle (struct MHD_Connection 
> *connection)
>      }
>    timeout = connection->connection_timeout;
>    if ( (timeout != 0) &&
> -       (timeout <= (time (NULL) - connection->last_activity)) )
> +       (timeout <= (MHD_monotonic_time() - connection->last_activity)) )
>      {
>        MHD_connection_close (connection, 
> MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
>        return MHD_YES;
> diff --git a/src/daemon/connection_https.c b/src/daemon/connection_https.c
> index 1330c3a..539b8de 100644
> --- a/src/daemon/connection_https.c
> +++ b/src/daemon/connection_https.c
> @@ -47,7 +47,7 @@ run_tls_handshake (struct MHD_Connection *connection)
>  {
>    int ret;
>  
> -  connection->last_activity = time (NULL);
> +  connection->last_activity = MHD_monotonic_time();
>    if (connection->state == MHD_TLS_CONNECTION_INIT)
>      {
>        ret = gnutls_handshake (connection->tls_session);
> @@ -138,7 +138,7 @@ MHD_tls_connection_handle_idle (struct MHD_Connection 
> *connection)
>              __FUNCTION__, MHD_state_to_string (connection->state));
>  #endif
>    timeout = connection->connection_timeout;
> -  if ( (timeout != 0) && (time (NULL) - timeout > connection->last_activity))
> +  if ( (timeout != 0) && (MHD_monotonic_time() - timeout > 
> connection->last_activity))
>      MHD_connection_close (connection,
>                         MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
>    switch (connection->state)
> diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
> index 29d7ced..451bb39 100644
> --- a/src/daemon/daemon.c
> +++ b/src/daemon/daemon.c
> @@ -546,7 +546,6 @@ MHD_get_fdset (struct MHD_Daemon *daemon,
>    return MHD_YES;
>  }
>  
> -
>  /**
>   * Main function of the thread that handles an individual
>   * connection when MHD_USE_THREAD_PER_CONNECTION is set.
> @@ -578,7 +577,7 @@ MHD_handle_connection (void *data)
>        tvp = NULL;
>        if (timeout > 0)
>       {
> -       now = time (NULL);
> +       now = MHD_monotonic_time();
>         if (now - con->last_activity > timeout)
>           tv.tv_sec = 0;
>         else
> @@ -938,7 +937,7 @@ MHD_add_connection (struct MHD_Daemon *daemon,
>    connection->addr_len = addrlen;
>    connection->socket_fd = client_socket;
>    connection->daemon = daemon;
> -  connection->last_activity = time (NULL);
> +  connection->last_activity = MHD_monotonic_time();
>  
>    /* set default connection handlers  */
>    MHD_set_http_callbacks_ (connection);
> @@ -1252,7 +1251,7 @@ MHD_get_timeout (struct MHD_Daemon *daemon,
>      }
>    if (!have_timeout)
>      return MHD_NO;
> -  now = time (NULL);
> +  now = MHD_monotonic_time();
>    if (earliest_deadline < now)
>      *timeout = 0;
>    else
> diff --git a/src/daemon/digestauth.c b/src/daemon/digestauth.c
> index 1e976bb..1f9547c 100644
> --- a/src/daemon/digestauth.c
> +++ b/src/daemon/digestauth.c
> @@ -619,7 +619,7 @@ MHD_digest_auth_check(struct MHD_Connection *connection,
>        
>      /* 8 = 4 hexadecimal numbers for the timestamp */  
>      nonce_time = strtoul(nonce + len - 8, (char **)NULL, 16);  
> -    t = (uint32_t) time(NULL);    
> +    t = (uint32_t) MHD_monotonic_time();    
>      /*
>       * First level vetting for the nonce validity if the timestamp
>       * attached to the nonce exceeds `nonce_timeout' then the nonce is
> @@ -751,7 +751,7 @@ MHD_queue_auth_fail_response(struct MHD_Connection 
> *connection,
>    char nonce[HASH_MD5_HEX_LEN + 9];
>  
>    /* Generating the server nonce */  
> -  calculate_nonce ((uint32_t) time(NULL),
> +  calculate_nonce ((uint32_t) MHD_monotonic_time(),
>                  connection->method,
>                  connection->daemon->digest_auth_random,
>                  connection->daemon->digest_auth_rand_size,
> diff --git a/src/daemon/internal.c b/src/daemon/internal.c
> index 3b0a544..453634e 100644
> --- a/src/daemon/internal.c
> +++ b/src/daemon/internal.c
> @@ -157,4 +157,14 @@ MHD_http_unescape (void *cls,
>    return wpos - val; /* = strlen(val) */
>  }
>  
> +time_t MHD_monotonic_time(void)
> +{
> +#ifdef HAVE_CLOCK_GETTIME
> +    struct timespec ts;
> +    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0)
> +     return ts.tv_sec;
> +#endif
> +    return time(NULL);
> +}
> +
>  /* end of internal.c */
> diff --git a/src/daemon/internal.h b/src/daemon/internal.h
> index 6e15af2..15900f1 100644
> --- a/src/daemon/internal.h
> +++ b/src/daemon/internal.h
> @@ -1057,5 +1057,11 @@ struct MHD_Daemon
>    (element)->next = NULL; \
>    (element)->prev = NULL; } while (0)
>  
> +/**
> + * Equivalent to time(NULL) but tries to use some sort of monotonic
> + * clock that isn't affected by someone setting the system real time
> + * clock.
> + */
> +time_t MHD_monotonic_time(void);
>  
>  #endif