Skip to Content.
Sympa Menu

freetds - Re: [freetds] dbsqlexec() never returns

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>
  • To: "FreeTDS Development Group" <freetds AT lists.ibiblio.org>
  • Subject: Re: [freetds] dbsqlexec() never returns
  • Date: Thu, 31 May 2007 17:14:32 +0200

>
> 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

yes, but doing so dbinterrupt cannot work

> removed the end_ms
> and diff_time code (lines 371 to 380), just leaving "if (ptv) break;".
>

quite complicated, yes... I noted some problems... if diff_time if 0
(which is quite probable) we do not exit but we set timeout to 0 which
can be considered infinite (this can be the cause of the infinite
loop!!). Also on EINTR we should compute again timeout. See my patch at
http://freetds.cvs.sourceforge.net/freetds/freetds/src/tds/net.c?r1=1.60
&r2=1.61&sortby=date, please consider a new snapshot (tomorrow one). I'd
be glad if you could test this.

> 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.
>

this was really, really broken, applied.

> Other changes I made were :
> Line 681 of net.c, for TDS_INT_TIMEOUT, I replaced the
> "continue" with a
> "return 0".

no, the idea is, on TDS_INT_TIMEOUT send cancel and wait reply from
server. On next timeout library should say, "ok, we waited quite a lot
after cancel, now stop and close". We can't return safely without
loosing syncronization.

> Line 689 of net.c, just before "p += nput" I added an error
> report and
> "return 0" if "nput <= 0".
>

Mmm... here nput <= 0 should be mathematically impossible...

> 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 */
> }
> }

Yes, it's ugly... and perhaps is useful only for EINTR !

> --- 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;

freddy77





Archive powered by MHonArc 2.6.24.

Top of Page