FreeTDS Development Group

["Coleman, Dave" <DColeman AT US.TIAuto.com>]

Warning, this is a very long email.  I have tried to make it comprehensive,
but I could not make it short, and so as not to ambush anyone, this deals
with a change to libtds that I believe is a very important feature which
might be worth implementing.  I am sure that I will rapidly have to close my
email in self-defense, but here goes nothing.

I have a class library (well, 3 classes, a tds wrapper, a recordset class,
and a row storage class) that can log in and out repeatedly without leaking
memory.  The connection to SQL server is dropped and opened - this has been
verified by a SQL Profiler trace.  Additionally, I made this work a while
ago around v0.52/0.53 when I was implementing tds.  We are now trying to use
the freetds library straight w/o custom code as part of a final clean up.

What I did is take the v0.53 tds_connect() and copy it into my class with
modifications that only allocate the login, config, and socket structures
once.  After the creation of the object, the TDSLOGIN structure is created
on the first and only first call to tdsDB::connectSock() (my method's name).
After that, the connection to the server is made by calling a call to
tdsDB::openDB(TDSLOGIN *login).  openDB() is called by connectSock() and
only connectSock() as the login is allocated by connectSock().  This was
done to allow for minimal code differences between openDB() and
tds_connect().  openDB() is basically tds_connect() with some mods to handle
the tdsDB::initialized flag that I use to track whether or not the tds
structures have had memory allocated.  As a matter of fact, I can only
identify two major differences in my function.  I dump out of the function
if my connected flag is set (I'd already have a connection in that case, so
the recordset class that uses the wrapper can have a re-usable open()
method).  I also only allocate the TDSSOCKET structure and the TDSCONFIGINFO
structure on the first call, before tdsDB::initialized is set.  I never call
tds_connect(), but I make a proper connection.  I don't use any
tds_free*whatever* function except in the delete method of my wrapper class
where I use tds_free_msg(), tds_free_all_results(), tds_free_socket(), and
tds_free_login().  To disconnect from the SQL server is make two calls (in
red and marked for those with no color):

class tdsDB
    {
    void closeDB(void);
    ...
    TDSSOCKET *sqlSock;
    int connected
    ...
    }

void tdsDB::closeDB(void)
    {
    if( connected )
        {
        shutdown(sqlSock->s,SHUT_RDWR);         <<-- call #1
        close(sqlSock->s);                              <<-- call #2
        connected = 0;
        }
    }

and with that the connection is dropped and ready for my modified version of
tds_connect() to happily re-log-in.  I will be more than happy to share my
work with the group and or the project, just ask for the code.
There are six files : 
        recordset.cpp/recordset.hpp 
        row.cpp/row.hpp and 
        tdsdb.cpp/tdsdb.hpp.

I'd really like to know if I've figured out something cool, if this is
something that could improve freeTDS, or if this is just a custom answer for
a designer app.  I can definately imagine that many people might want to
have a non-persistant connection to an object and not have to leak memory to
do it.  We have a custom windowing library that I've used for the error
message handler, I could try to make a universal version of my classes if
desired, but I think that we allow other developers more flexibility by
keeping everything non-class based on the libtds level. 

I will admit flat out that this precludes the option of having the
connection object be able to dynamically react to server configuration
changes (without leaking 3062 bytes each time an object is deleted), but
based on the info stored in the TDSLOGIN and TDSCONFIGINFO structures, I
kind of doubt that these values will change often in any environment, and if
they are, then it is probably server setup time, and this limitation is only
within the one instance of the object.  Any program can be ended and
restarted to refresh server config info.  It could be possible to refresh
some of this on the fly if we can store and re-use the existing HOSTENT *
pointer returned byt the first call to gethostbyname() and not re-execute
gethostbyname every time we want config stuff.  Again, this limits network
reactivity, but this should not ever really matter.  (Excepting times of
devlopment and perhaps laptops that can roam across multiple wireless
networks - which presents many more complications which are much more
detrimental than this).  Besides in times of development, most systems go up
and down so often that this would not really be an issue.  Maybe I'm wrong
about that.  If so please say so, but I always like to do fresh-boot testing
when network parameters have changed.  It's just good practice to sanity
check the whole system from boot up before unleashing it on the rest of the
world.


Here's some code:  I was thinking the following for mappings in libtds...
(my fn --> proposed libtds fn)  Obviously, my coding style does not match
that of the project, I appologize.

tdsDB::connectSock() --> tds_conn_open(TDSLOGIN *tds_login, TDSSOCKET
*tds_socket)
        this function would call tds_alloc_login if needed and then call
either modified tds_connect() or new non-persistant function

tdsDB::openDB() --> tds_connect(...) or tds_conn_bind(...)
        use the same parameters as tds_connect, and handle the
non-persistant memory allocation stuff, or make new function (tds_conn_bind)
to not break old systems.  I'm not sure what the overall effect on non-C-API
users would be if tds_connect changed this way, but I have fuzzy visions of
it being problematic so maybe it should be a new function.

tdsDB::closeDB() --> tds_disconnect(TDSSOCKET *tds_socket)
        just use the shutdown and close calls (see code below).

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*
// -|> Mrdegdc  7/26/2001  1:43pm
//
// use this function to open a connection to your
// data source.
//--------------------------------------------------
int tdsDB::connectSock( void )
    {
    rows = 0;
    if( !initialized )
        {
        sqlLogin = tds_alloc_login();
        }
    if( !sqlLogin )
        {
        return TDS_FAIL;
        }
    tds_set_passwd(sqlLogin, pwd);
    tds_set_user(sqlLogin, user);
    tds_set_app(sqlLogin, hostName);
    tds_set_host(sqlLogin, hostName );
    tds_set_library(sqlLogin, "TDS-Library");
    tds_set_server(sqlLogin, server );
    tds_set_charset(sqlLogin, "iso_1");
    tds_set_language(sqlLogin, "us_english");
    tds_set_packet(sqlLogin, 512);
    if( !isConnected() )
        {
        sqlSock = openDB(sqlLogin);
        }
    if( ! sqlSock )
        {
        loggedIn = 0;
        offLine = 1;
        return SQL_NO_SOCKET;
        }
    loggedIn = 1;
      offLine = 0;
    return TDS_SUCCEED;
    }

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*
// -|> Mrdegdc  7/26/2001  1:52pm
//
// this function sets the state tags for the
// class object to indicate an active dababase
// connection in addition to opening a socket
// to the server.
//----------------------------------------------
TDSSOCKET* tdsDB::openDB(TDSLOGIN *login)
    {
    TDSSOCKET   *tds = NULL;
    struct sockaddr_in      sin;
    unsigned long ioctl_blocking = 1;
    struct timeval selecttimeout;
    fd_set fds;
    int retval;
    time_t start, now;
    /* 13 + max string of 32bit int, 30 should cover it */
    char query[30];

    if( !connected )
        {
        FD_ZERO (&fds);
        if( !initialized )
            {

 
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*
            // -|> Mrdegdc  9/7/2001  8:24am
            //
            // The initial login packet must have a block size of 512.
            // Once the connection is established the block size can be
changed
            // by the server with TDS_ENV_CHG_TOKEN, need method to do
that...
 
//------------------------------------------------------------------

            tds = tds_alloc_socket(512);
            config = tds_get_config(tds, sqlLogin,NULL);
            initialized = 1;
            }
        else
            {
            tds=sqlSock;
            }

        tds->major_version=config->major_version;
        tds->minor_version=config->minor_version;
        tds->emul_little_endian=config->emul_little_endian;

        /* Jeff's hack - begin */
        tds->timeout = (login->connect_timeout) ? login->query_timeout : 0;
        tds->longquery_timeout = (login->connect_timeout) ?
login->longquery_timeout : 0;
        tds->longquery_func = login->longquery_func;
        tds->longquery_param = login->longquery_param;
        /* end */

        sin.sin_addr.s_addr = inet_addr(config->ip_addr);
        sin.sin_family = AF_INET;
        sin.sin_port = htons(config->port);
        memcpy(tds->capabilities,login->capabilities,TDS_MAX_CAPABILITY);
        tds->s = socket (AF_INET, SOCK_STREAM, 0);
        if( tds->s < 0 )
            {
            perror ("socket line 312");
            return NULL;
            }
        /* Jeff's hack *** START OF NEW CODE *** */
        if( login->connect_timeout )
            {
            start = time (NULL);
            ioctl_blocking = 1; /* ~0; //TRUE; */
            if( IOCTL(tds->s, FIONBIO, &ioctl_blocking) < 0 )
                {
                return NULL;
                }
            connect(tds->s, (struct sockaddr *) &sin, sizeof(sin));
            /* Select on writeability for connect_timeout */
            selecttimeout.tv_sec = 0;
            selecttimeout.tv_usec = 0;

            FD_SET (tds->s, &fds);
            retval = 0;
            retval = select(tds->s + 1, NULL, &fds, NULL, &selecttimeout);
            /* patch from Kostya Ivanov <kostya AT warmcat.excom.spb.su> */
            if( retval < 0 && errno == EINTR )
                retval = 0;
            /* end patch */

            now = time (NULL);

            while( (retval == 0) && ((now-start) < login->connect_timeout) )
                {
                tds_msleep(1);
                FD_SET (tds->s, &fds);
                selecttimeout.tv_sec = 0;
                selecttimeout.tv_usec = 0;
                retval = select(tds->s + 1, NULL, &fds, NULL,
&selecttimeout);
                now = time (NULL);
                }

            if( (now-start) > login->connect_timeout )
                {
                return NULL;
                }
            }
        else
            {
            if( connect(tds->s, (struct sockaddr *) &sin, sizeof(sin)) <0 )
                {
                perror("connect");
                return NULL;
                }
            }
        /* END OF NEW CODE */

        tds->out_flag=0x02;
        tds_send_login(tds,config);

        if( !tds_process_login_tokens(tds) )
            {
            tds_free_config(config);
            tds_free_socket(tds);
            free(sqlLogin);
            tds = NULL;
            offLine = 0;
            initialized = 0;
            }
        if( tds && config->text_size )
            {
            sprintf(query,"set textsize %d", config->text_size);
            retval = tds_submit_query(tds,query);
            if( retval == TDS_SUCCEED )
                {
                while( tds_process_result_tokens(tds)==TDS_SUCCEED );
                }
            }
        }
    if( tds )
        {
        loggedIn = 1;
        offLine = 0;
        connected = 1;
        }
    return tds;
    }

void tdsDB::closeDB(void)
    {
    if( connected )
        {
        shutdown(sqlSock->s,SHUT_RDWR);
        close(sqlSock->s);
        connected = 0;
        }
    }



Let me take that back, I ran dmalloc on the thing (btw, I've modified all 
the code in src/tds to include

#ifdef DMALLOC
#include <dmalloc.h>
#endif

). Anyway, I found a free'ing of a NULL pointer in mem.c, but otherwise we 
are clean with the current CVS code.  What i did find is that 
gethostbyname() and friends are doing some allocation (3062 bytes) that 
isn't getting released, but that is libc stuff, not our concern, and that 
number stays the same whether it's 10 iterations or 10,000.

So dave, let me know if you are still having problems and if you are, how 
you are gauging the memory leak.

Brian

Happy July 4th to everyone in the states.  To everyone else, sorry you 
have to work today. ;-)  Four day weekend, woohoo!