Fix for ODBC close

Bruce Momjian writes:

I have applied the following patch to properly exit ODBC. I also
patched the ODBC makefile so it links under BSD/OS. The -Bsymbolic
under BSD/OS is very harsh under BSD/OS, requiring all symbols even in
libc and crt1.o to be resolved before creating the shared library.

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/

Bruce Momjian writes:

I have applied the following patch to properly exit ODBC. I also
patched the ODBC makefile so it links under BSD/OS. The -Bsymbolic
under BSD/OS is very harsh under BSD/OS, requiring all symbols even in
libc and crt1.o to be resolved before creating the shared library.

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

OK, if this is true on all platforms, why isn't -lc needed?

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

I have applied the following patch to properly exit ODBC. I also
patched the ODBC makefile so it links under BSD/OS. The -Bsymbolic
under BSD/OS is very harsh under BSD/OS, requiring all symbols even in
libc and crt1.o to be resolved before creating the shared library.

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

OK, if this is true on all platforms, why isn't -lc needed?

And if -lc is somehow done by default with ld -Bsymbolic, how do I deal
with a link that accesses crt1.o startup symbols, like environ and
__progname?

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

I have applied the following patch to properly exit ODBC. I also
patched the ODBC makefile so it links under BSD/OS. The -Bsymbolic
under BSD/OS is very harsh under BSD/OS, requiring all symbols even in
libc and crt1.o to be resolved before creating the shared library.

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

OK, if this is true on all platforms, why isn't -lc needed?

And if -lc is somehow done by default with ld -Bsymbolic, how do I deal
with a link that accesses crt1.o startup symbols, like environ and
__progname?

OK, the following fixes the link on BSDI, while allowing -Bsymbolic. I
have to explicitly include -R crt1.o to be used to resolve symbols, but
not to be linked in. Without -R, I get undefined 'main' which makes
sense.

I am still confused why other OS's work, unless -lc is assumed by ld,
and their libc's have no crt1.o references.

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

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

OK, if this is true on all platforms, why isn't -lc needed?

Theory 1: Many other platforms use the compiler driver ([g]cc) to link
shared libraries. That makes all the right things happen. Most likely
this should happen on a lot more platforms that currently use ld directly.

Theory 2: Not many people have tried to build the ODBC driver on
non-mainstream platforms.

--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/

Bruce Momjian writes:

The -Bsymbolic switch is the same on all platforms that have it. You can
link without it, but then you won't actually be able to use the ODBC
driver. It seems like you need to link in a few other libraries to
resolve all symbols.

OK, if this is true on all platforms, why isn't -lc needed?

Theory 1: Many other platforms use the compiler driver ([g]cc) to link
shared libraries. That makes all the right things happen. Most likely
this should happen on a lot more platforms that currently use ld directly.

Theory 2: Not many people have tried to build the ODBC driver on
non-mainstream platforms.

I just tried gcc and got:

#$ gcc -shared -soname libpsqlodbc.so.0 -Bsymbolic info.o bind.o
columninfo.o connection.o convert.o drvconn.o environ.o execute.o lobj.o
misc.o options.o pgtypes.o psqlodbc.o qresult.o results.o socket.o
parse.o statement.o gpps.o tuple.o tuplelist.o dlg_specific.o
-L/usr/local/lib -L/usr/contrib/lib -lm -o libpsqlodbc.so.0.26
gcc: unrecognized option `-soname'
gcc: file path prefix `symbolic' never used

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

I just tried gcc and got:

#$ gcc -shared -soname libpsqlodbc.so.0 -Bsymbolic info.o bind.o
columninfo.o connection.o convert.o drvconn.o environ.o execute.o lobj.o
misc.o options.o pgtypes.o psqlodbc.o qresult.o results.o socket.o
parse.o statement.o gpps.o tuple.o tuplelist.o dlg_specific.o
-L/usr/local/lib -L/usr/contrib/lib -lm -o libpsqlodbc.so.0.26
gcc: unrecognized option `-soname'
gcc: file path prefix `symbolic' never used

Try gcc -shared -Wl,-soname,libpsqlodbc.so.0 -Wl,-Bsymbolic ...

--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/

Bruce Momjian writes:

I just tried gcc and got:

#$ gcc -shared -soname libpsqlodbc.so.0 -Bsymbolic info.o bind.o
columninfo.o connection.o convert.o drvconn.o environ.o execute.o lobj.o
misc.o options.o pgtypes.o psqlodbc.o qresult.o results.o socket.o
parse.o statement.o gpps.o tuple.o tuplelist.o dlg_specific.o
-L/usr/local/lib -L/usr/contrib/lib -lm -o libpsqlodbc.so.0.26
gcc: unrecognized option `-soname'
gcc: file path prefix `symbolic' never used

Try gcc -shared -Wl,-soname,libpsqlodbc.so.0 -Wl,-Bsymbolic ...

OK, this works:

gcc -shared -Wl,-Bsymbolic,-soname,libpsqlodbc.so.0 info.o bind.o
columninfo.o connection.o convert.o drvconn.o environ.o execute.o lobj.o
misc.o options.o pgtypes.o psqlodbc.o qresult.o results.o socket.o
parse.o statement.o gpps.o tuple.o tuplelist.o dlg_specific.o
-L/usr/local/lib -L/usr/contrib/lib -lm -lc -o libpsqlodbc.so.0.26

I replaced the 'ld' with 'gcc -Wl', and that prevents the need for the
crt1.o.

It still requires -lc:

ifneq ($(PORTNAME), bsdi)
LINK.shared += $(shlib_symbolic)
else
LINK.shared = gcc -shared -Wl,-Bsymbolic,-soname,$(soname)
SHLIB_LINK += -lc
endif

It seems the -Bsymbolic needs the gcc, while other links are OK with ld.
We may find this is true on many platforms.

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

You can't hardcode "gcc" like that. I've committed some fixes that should
work for you. Please try them out. Also try to build libpq++.

It seems the -Bsymbolic needs the gcc, while other links are OK with ld.
We may find this is true on many platforms.

-Bsymbolic requires all symbols in the library to be resolvable at link
time. If you use 'ld' then you will need to provide all the appropriate
files yourself. The compiler driver normally does that automatically.

Great. I see you modified Makefile.bsdi to properly know it is being
used with gcc, and modified Makefile.shlib. Perfect.

Should other platforms have this fix too? We didn't need it before
-Bsymbolic, but it seems it would be safe to do for FreeBSD and a few
others.

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

I replaced the 'ld' with 'gcc -Wl', and that prevents the need for the
crt1.o.

It still requires -lc:

ifneq ($(PORTNAME), bsdi)
LINK.shared += $(shlib_symbolic)
else
LINK.shared = gcc -shared -Wl,-Bsymbolic,-soname,$(soname)
SHLIB_LINK += -lc
endif

You can't hardcode "gcc" like that. I've committed some fixes that should
work for you. Please try them out. Also try to build libpq++.

It seems the -Bsymbolic needs the gcc, while other links are OK with ld.
We may find this is true on many platforms.

-Bsymbolic requires all symbols in the library to be resolvable at link
time. If you use 'ld' then you will need to provide all the appropriate
files yourself. The compiler driver normally does that automatically.

--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/

-Bsymbolic requires all symbols in the library to be resolvable at link
time. If you use 'ld' then you will need to provide all the appropriate
files yourself. The compiler driver normally does that automatically.

Great. I see you modified Makefile.bsdi to properly know it is being
used with gcc, and modified Makefile.shlib. Perfect.

Should other platforms have this fix too? We didn't need it before
-Bsymbolic, but it seems it would be safe to do for FreeBSD and a few
others.

I have applied the following patch for OpenBSD and FreeBSD. They have
the same -Bsymbolic handling and same use of LD for linking. I made the
duplicate changes Peter made for BSDI.

Can anyone commend on the use of 'ld -x' to delete all local symbols?
FreeBSD and OpenBSD have it, while BSD/OS does not. I added it to BSDi,
and it seems to work fine.

Actually, it seems NetBSD already had all these fixes.

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

I have applied the following patch for OpenBSD and FreeBSD. They have
the same -Bsymbolic handling and same use of LD for linking. I made the
duplicate changes Peter made for BSDI.

Hmm, at least on OpenBSD the recommended way to build shared libraries is
using 'ld' directly. But using gcc should work as well.

Can anyone commend on the use of 'ld -x' to delete all local symbols?
FreeBSD and OpenBSD have it, while BSD/OS does not. I added it to BSDi,
and it seems to work fine.

I don't think it should be used.

Actually, it seems NetBSD already had all these fixes.

On NetBSD, there are about 4 different ways of build shared libraries,
depending on version and platform. Nothing I wanna mess with.

--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/

pgman@candle.pha.pa.us wrote:

I have applied the following patch to properly exit ODBC. I also
patched the ODBC makefile so it links under BSD/OS. The -Bsymbolic
under BSD/OS is very harsh under BSD/OS, requiring all symbols even in
libc and crt1.o to be resolved before creating the shared library.

My 'ld' manual says:

-Bsymbolic
When creating a shared library, bind references to
global symbols to the definition within the shared
library, if any. Normally, it is possible for a
program linked against a shared library to override
the definition within the shared library. This op-
tion is only meaningful on ELF platforms which sup-
port shared libraries.

Hmm,

removing that may break it when running under a driver manager though...

I will check of FreeBSD, it certainly will on Linux ELF.

--
Nick Gorham
When I die, I want to go like my grandfather did, gently while sleeping,
and not like his passangers, screaming in a panic, looking for the
inflatable raft. -- Seen on ./

Bruce Momjian writes:

I have applied the following patch for OpenBSD and FreeBSD. They have
the same -Bsymbolic handling and same use of LD for linking. I made the
duplicate changes Peter made for BSDI.

Hmm, at least on OpenBSD the recommended way to build shared libraries is
using 'ld' directly. But using gcc should work as well.

Can anyone commend on the use of 'ld -x' to delete all local symbols?
FreeBSD and OpenBSD have it, while BSD/OS does not. I added it to BSDi,
and it seems to work fine.

I don't think it should be used.

Can someone comment on why people would have added that?

Actually, it seems NetBSD already had all these fixes.

On NetBSD, there are about 4 different ways of build shared libraries,
depending on version and platform. Nothing I wanna mess with.

Yes, BSDI has even more, but I think we are now doing the same thing on
all the bsd's. Interesting that NetBSD was the only "right" one.

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

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

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

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

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

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

+               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

-----Original Message-----
From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
Sent: 11 February 2001 06:31
To: PostgreSQL-development
Cc: Dave Page; PostgreSQL odbc list; PostgreSQL-patches
Subject: Re: [ODBC] RE: [PATCHES] Fix for ODBC close

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.

Sounds reasonable though I must admit this isn't exactly my area of
expertise! I'll certainly test any patches though and make a .dll available
for others to try.

Regards,

Dave.

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