This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] sunrpc/clnt_udp.c: always respect timeout(s)

On 06/03/2016 11:26 AM, Pavel Raiskup wrote:

Previously, when the port we were listening on was really busy,
clntudp_call () ended up processing the UDP queue without properly
defined time-limit (clntudp_call could keep processing forever
without returning back to caller).

The fix incorporates using of clock_gettime () to measure the real
timeout, instead of simply counting the number of poll()'s.  We
should not face some huge slowdown because we call clock_gettime
only twice in most cases *and* never in busy loop.


I field a bug to track this:

The commit message and ChangeLog need to reference this (as “[BZ #2057]”).

diff --git a/sunrpc/clnt_udp.c b/sunrpc/clnt_udp.c
index 6ffa5f2..2cab4c2 100644
--- a/sunrpc/clnt_udp.c
+++ b/sunrpc/clnt_udp.c
@@ -1,6 +1,8 @@
  * clnt_udp.c, Implements a UDP/IP based, client side RPC.
+ * Copyright (c) 2016, Free Software Foundation, Inc.
+ *
  * Copyright (c) 2010, Oracle America, Inc.

Please do not change the copyright notice in this way. Either add the full LGPL header, or leave it unchanged.

+static bool_t
+clntudp_call_timeouted (const struct timespec *start, int timeout, int *resend_timeout)
+  struct timespec now;
+  long remains, waited;
+  __clock_gettime (CLOCK_MONOTONIC, &now);

I was mistaken about the availability of CLOCK_MONOTONIC. It seems to be missing on Hurd. CLOCK_REALTIME is probably missing there as well, so we need fallback to gettimeofday.

We have similar code in resolv/res_send.c. It needs to use CLOCK_MONOTONIC if available, but currently does not. So this should really move to generic libc functionality (for separate testing, too).

I can take care of this if you want. Unless you want to learn a bit about glibc symbol management.

+  /* count everything in milliseconds */
+  waited = (now.tv_sec  - start->tv_sec  - 1)          * 1000
+         + (now.tv_nsec - start->tv_nsec + 1000000000) / 1000000;
+  remains = timeout - waited;
+  if (*resend_timeout > remains)
+    *resend_timeout = remains;
+  return (*resend_timeout <= 0);

This should perform a bit of overflow checking, or use unsigned long long at least. It probably won't matter in practice because 2**31 milliseconds is a long time, but let's cover this properly.

From an interface perspective, it seems preferable to compute an explicit deadline, and count the milliseconds towards that, rather than recomputing the absolute deadline from the millisecond value.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]