FreeTDS Development Group

[Perception Technologies Ltd <ptl AT perceptions.co.nz>]

Hi,

We use the FreeTDS C library on a Solaris 10 system to send a call to a 
database procedure on a SQL Server database on a Windoze box.  We were 
using the 0.63 stable release, but were getting an occasional (~ once per 2 
months) timeout issue whereby the dbsqlexec() function would never 
return.  With the recent improvements to timeout we changed to the 
development release 0.65 from 9th January 2007.

All of our testing worked fine, but when we went live with it, we got the 
dbsqlexec() function not returning about once a day.  Turning on debugging 
showed the problem to be with tds_goodwrite().

I looked at the code, and saw what I thought could be several problems.  So 
I made a few changes, and added some extra debugging, and installed the new 
version.  We have not had a problem since (10 days now).  Unfortunately, I 
am unable to ascertain which of the changes I made resolved the actual 
problem that we had, because the problem only occurs intermittently on the 
live system, and not on the test system.  So I thought I would detail the 
changes that were made so that someone else may be able to make use of them.

Firstly, the Solaris man page for select(3c) states that if a select() call 
times out, then the three fds parameters have their bits cleared.  In the 
FreeTDS tds_select() function (in net.c), the select() call is in a loop 
with a 1 second time out, so if the socket is not available for writing 
within the first second, then it will pointlessly wait the remaining period 
of the configuration timeout, and then timeout.  I fixed this by always 
setting poll_seconds to timeout_seconds (line 334), and removed the end_ms 
and diff_time code (lines 371 to 380), just leaving "if (ptv) break;".

Secondly, at line 659 of net.c, the result of the send() (#ifdef 
USE_MSGMORE) is stored in a variable called "nput", but then a variable 
called "len" is checked, and reset on the next two lines.  I changed "len" 
to "nput" on these two lines.

Other changes I made were :
Line 681 of net.c, for TDS_INT_TIMEOUT, I replaced the "continue" with a 
"return 0".
Line 689 of net.c, just before "p += nput" I added an error report and 
"return 0" if "nput <= 0".

A diff of those changes is attached.

Hope this is of use.

Cheers,
        Brent.

*** old/net.c   Tue Jan  9 05:18:41 2007 UTC
--- new/net.c   Thu May 21 04:37:49 2007 UTC
***************
*** 321,324 ****
--- 321,326 ----
  {
    int rc, seconds;
+ // brent 21-May-07 - Always select for full timeout
+   const unsigned int poll_seconds = timeout_seconds;

    assert(tds != NULL);
***************
*** 332,341 ****
     */
    seconds = timeout_seconds;
-   const unsigned int poll_seconds = (tds->tds_ctx && 
tds->tds_ctx->int_handler)? 1 : timeout_seconds;
    assert(seconds >= 0);
    do {
        struct timeval tv, *ptv = timeout_seconds? &tv : NULL;
-       unsigned int end_ms = poll_seconds * 1000 + tds_gettime_ms();
-
        /*
         * The poll loop. We exit on the first of these events:
--- 334,340 ----
***************
*** 369,382 ****

            if (ptv) {
-               const int diff_time = (int) (end_ms - tds_gettime_ms());
-
-               if (diff_time < 0)
                    break;
-
-               tv.tv_sec  = diff_time / 1000;
-               tv.tv_usec = (diff_time % 1000) * 1000;
-
-               if (tv.tv_sec > poll_seconds || tv.tv_usec > 1000000)
-                   break;  /* shouldn't happen; start again */
            }
        }
--- 368,372 ----
***************
*** 657,662 ****
                nput = send(tds->s, p, remaining, last ? MSG_NOSIGNAL : 
MSG_NOSIGNAL|MSG_MORE);
                /* In case the kernel does not support MSG_MORE, try again 
without it */
!               if (len < 0 && errno == EINVAL && !last)
!                   len = send(tds->s, p, remaining, MSG_NOSIGNAL);
  #elif !defined(MSG_NOSIGNAL)
                nput = WRITESOCKET(tds->s, p, remaining);
--- 647,653 ----
                nput = send(tds->s, p, remaining, last ? MSG_NOSIGNAL : 
MSG_NOSIGNAL|MSG_MORE);
                /* In case the kernel does not support MSG_MORE, try again 
without it */
! // brent 21-May-07 - len should be nput on the next two lines
!               if (nput < 0 && errno == EINVAL && !last)
!                   nput = send(tds->s, p, remaining, MSG_NOSIGNAL);
  #elif !defined(MSG_NOSIGNAL)
                nput = WRITESOCKET(tds->s, p, remaining);
***************
*** 679,683 ****
            case TDS_INT_TIMEOUT:
                tds_send_cancel(tds);
!               continue; /* fixme: or return? */
            default:
            case TDS_INT_CANCEL:
--- 670,675 ----
            case TDS_INT_TIMEOUT:
                tds_send_cancel(tds);
! // brent 21-May-07 - Return fail seems more sensible then continue
!               return 0;
            default:
            case TDS_INT_CANCEL:
***************
*** 687,690 ****
--- 679,689 ----
            assert(0); /* not reached */
        }
+ // brent 21-May-07 If nothing sent, then abort (avoid a possible infinite 
loop)
+       if ( nput <= 0 )
+       {
+           tdsdump_log(TDS_DBG_ERROR, "Send sent %d bytes (of %d)\n",
+                   nput, remaining);
+           return 0;
+       }

        p += nput;