Thread: RE: [PATCHES] Fix for ODBC close

RE: [PATCHES] Fix for ODBC close

From
Dave Page
Date:

> -----Original Message-----
> From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> Sent: 10 February 2001 05:46
> To: PostgreSQL odbc list; PostgreSQL-patches
> Cc: PostgreSQL-development
> Subject: [PATCHES] Fix for ODBC close
>
>
> I have applied the following patch to properly exit ODBC.

<Snip>

I just compiled from the current cvs under win32 and I still get
'pq_recvbuf: unexpected EOF on client connection' when exiting apps using
the ODBC driver. I have tested with pgAdmin which uses ADO (ActiveX Data
Objects) and certainly closes the ADO connection object on exit, as well as
a simple test app using DAO (Data Access Objects). I did have a go at fixing
this myself when Bruce first mentioned it, and had exactly the same results
with similar code :-(

Regards,

Dave.

Re: RE: [PATCHES] Fix for ODBC close

From
Bruce Momjian
Date:
OK, I have ifdef'ed out the sending of the 'X' parameter.  I will see if
placing it somewhere else will help.  Could it have to do with the fact
we are in a transaction in ODBC?  My guess is that the X is returning
data that is triggering the error.

[ Charset ISO-8859-1 unsupported, converting... ]
>
>
> > -----Original Message-----
> > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> > Sent: 10 February 2001 05:46
> > To: PostgreSQL odbc list; PostgreSQL-patches
> > Cc: PostgreSQL-development
> > Subject: [PATCHES] Fix for ODBC close
> >
> >
> > I have applied the following patch to properly exit ODBC.
>
> <Snip>
>
> I just compiled from the current cvs under win32 and I still get
> 'pq_recvbuf: unexpected EOF on client connection' when exiting apps using
> the ODBC driver. I have tested with pgAdmin which uses ADO (ActiveX Data
> Objects) and certainly closes the ADO connection object on exit, as well as
> a simple test app using DAO (Data Access Objects). I did have a go at fixing
> this myself when Bruce first mentioned it, and had exactly the same results
> with similar code :-(
>
> Regards,
>
> Dave.
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: RE: [PATCHES] Fix for ODBC close

From
Bruce Momjian
Date:
OK, I have a pretty good guess about the cause of the ODBC shutdown
failure message in the server logs.  Sending 'X' is still causing the
error message.

The error you are seeing is from the backend libpq code, the area that
communicates with clients.

So, let's assume the problem is not the platform, but the client code.
Libpq properly shuts connections without triggering that message.  ODBC
does trigger the message.

libpq closes connections with:

        (void) pqPuts("X", conn);
        (void) pqFlush(conn);

while ODBC closes with:

        SOCK_put_char(self, 'X');
        SOCK_flush_output(self);

They then close() the socket.

It seems the difference is in the flushing.  libpq has elaborate flush
code:

    while (len > 0)
    {
            sent = send(conn->sock, ptr, len, 0);
            len -= sent;

            if (pqWait(FALSE, TRUE, conn))
    }

and pqWait does:

        if (select(conn->sock + 1, &input_mask, &output_mask, (fd_set *) NULL,


For flush, ODBC does a simple:

    written = send(self->socket, (char *) self->buffer_out, self->buffer_filled_out, 0);


It seems we may need to add flush code similar to libpq in ODBC.

At a minimum, we have to put the send() in a loop and keep going until
there are no more bytes to send.  Not sure the select() is required.

Comments?

After I receive comments, I will prepare a patch people can test.


---------------------------------------------------------------------------



.  > OK, I have ifdef'ed out the sending of the 'X' parameter.  I will see if
> placing it somewhere else will help.  Could it have to do with the fact
> we are in a transaction in ODBC?  My guess is that the X is returning
> data that is triggering the error.
>
> [ Charset ISO-8859-1 unsupported, converting... ]
> >
> >
> > > -----Original Message-----
> > > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> > > Sent: 10 February 2001 05:46
> > > To: PostgreSQL odbc list; PostgreSQL-patches
> > > Cc: PostgreSQL-development
> > > Subject: [PATCHES] Fix for ODBC close
> > >
> > >
> > > I have applied the following patch to properly exit ODBC.
> >
> > <Snip>
> >
> > I just compiled from the current cvs under win32 and I still get
> > 'pq_recvbuf: unexpected EOF on client connection' when exiting apps using
> > the ODBC driver. I have tested with pgAdmin which uses ADO (ActiveX Data
> > Objects) and certainly closes the ADO connection object on exit, as well as
> > a simple test app using DAO (Data Access Objects). I did have a go at fixing
> > this myself when Bruce first mentioned it, and had exactly the same results
> > with similar code :-(
> >
> > Regards,
> >
> > Dave.
> >
>
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

RE: RE: [PATCHES] Fix for ODBC close

From
"Hiroshi Inoue"
Date:
> -----Original Message-----
> From: Bruce Momjian
>
> OK, I have a pretty good guess about the cause of the ODBC shutdown
> failure message in the server logs.  Sending 'X' is still causing the
> error message.
>
> The error you are seeing is from the backend libpq code, the area that
> communicates with clients.
>
> while ODBC closes with:
>
>         SOCK_put_char(self, 'X');
>         SOCK_flush_output(self);
>

Probably you have to put above code before calling
shutdown() not after.  shutdown(sock, 2) disables
both sends and receives on the socket.

Regards,
Hiroshi Inoue


Re: RE: [PATCHES] Fix for ODBC close

From
Bruce Momjian
Date:
> > -----Original Message-----
> > From: Bruce Momjian
> >
> > OK, I have a pretty good guess about the cause of the ODBC shutdown
> > failure message in the server logs.  Sending 'X' is still causing the
> > error message.
> >
> > The error you are seeing is from the backend libpq code, the area that
> > communicates with clients.
> >
> > while ODBC closes with:
> >
> >         SOCK_put_char(self, 'X');
> >         SOCK_flush_output(self);
> >
>
> Probably you have to put above code before calling
> shutdown() not after.  shutdown(sock, 2) disables
> both sends and receives on the socket.

Thanks.  I was so focused on close() I never noticed the shutdown().
Can someone please test now?

Hiroshi, should I be concerned that a send() that does not send the full
packet just returns an error and does not retry?  Is libpq() so complex
because of async connections?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [HACKERS] Re: RE: [PATCHES] Fix for ODBC close

From
Tom Lane
Date:
+               SOCK_put_char(self, 'X');
+               SOCK_flush_output(self);
+               if (!shutdown(self->socket, 2)) /* no sends or receives */
                        closesocket(self->socket);

I think you should issue the close() whether the shutdown() succeeds or
not.  Otherwise you have a file descriptor leak.  In fact, given that
you're going to close the socket, the separate shutdown call is a
complete waste of cycles.  Take it out.

> Hiroshi, should I be concerned that a send() that does not send the full
> packet just returns an error and does not retry?  Is libpq() so complex
> because of async connections?

Right, libpq only needs to loop because it runs the socket in nonblock
mode.  SOCK_flush_output looks OK to me.  (SOCK_get_next_byte, on the
other hand, goes wacko on error or close... probably should make it
return a null character instead of random data.)

            regards, tom lane

Re: RE: [PATCHES] Fix for ODBC close

From
Hiroshi Inoue
Date:
Bruce Momjian wrote:
>
> > > -----Original Message-----
> > > From: Bruce Momjian
> > >
> > > OK, I have a pretty good guess about the cause of the ODBC shutdown
> > > failure message in the server logs.  Sending 'X' is still causing the
> > > error message.
> > >
> > > The error you are seeing is from the backend libpq code, the area that
> > > communicates with clients.
> > >
> > > while ODBC closes with:
> > >
> > >             SOCK_put_char(self, 'X');
> > >             SOCK_flush_output(self);
> > >
> >
> > Probably you have to put above code before calling
> > shutdown() not after.  shutdown(sock, 2) disables
> > both sends and receives on the socket.
>
> Thanks.  I was so focused on close() I never noticed the shutdown().
> Can someone please test now?
>

I had already tested it in win32 environment before
I posted my previous mail.

Regards,
Hiroshi Inoue