BUG #1545: LIBPQ Windows Version not calling WSACleanup for every WSAStartup

Jason Erickson wrote:

Taken from microsoft at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winsock/win
sock/wsastartup_2.asp
-------------------------------------------------------
An application must call one WSACleanup call for every successful WSAStartup
call to allow third-party DLLs to make use of a WS2_32.DLL on behalf of an
application. This means, for example, that if an application calls
WSAStartup three times, it must call WSACleanup three times. The first two
calls to WSACleanup do nothing except decrement an internal counter; the
final WSACleanup call for the task does all necessary resource deallocation
for the task.
------------------------------------------------------------------

The only place WSACleanup is being called is libpqdll when the process
detaches the DLL (if the libpq is not staticly linked in), which matches up
with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does not have a
matching WSACleanup. WSACleanup could possibly be placed in freePGconn(),
but unsure if all possible error cases will go through this function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq interface for
Windows.

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

Go ahead and put it in --- I'll wait for your commit.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Jason Erickson wrote:

The only place WSACleanup is being called is libpqdll when the process
detaches the DLL (if the libpq is not staticly linked in), which matches up
with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does not have a
matching WSACleanup. WSACleanup could possibly be placed in freePGconn(),
but unsure if all possible error cases will go through this function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq interface for
Windows.

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

Actually ... why do we need a WSAStartup in makeEmptyPGconn? If we have
one in DLL attach, isn't that sufficient?

regards, tom lane

The only place WSACleanup is being called is libpqdll when

the process

detaches the DLL (if the libpq is not staticly linked in),

which matches up

with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does

not have a

matching WSACleanup. WSACleanup could possibly be placed

in freePGconn(),

but unsure if all possible error cases will go through this

function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq

interface for

Windows.

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

Actually ... why do we need a WSAStartup in makeEmptyPGconn?
If we have
one in DLL attach, isn't that sufficient?

Not if you link libpq as a static lib...

I *think* that's why it was added. In the beginning it was only in the
DLL attach code, and if you were using the library as a static lib you
had to call it yourself. Not sure when that changed, but clearly that is
when the WSACleanup() call should've gone in.

//Magnus

"Magnus Hagander" <mha@sollentuna.net> writes:

Actually ... why do we need a WSAStartup in makeEmptyPGconn?
If we have one in DLL attach, isn't that sufficient?

Not if you link libpq as a static lib...

I *think* that's why it was added. In the beginning it was only in the
DLL attach code, and if you were using the library as a static lib you
had to call it yourself. Not sure when that changed, but clearly that is
when the WSACleanup() call should've gone in.

Ah. Makes sense. So adding it to freePGconn is correct.

Bruce, you should probably back-patch for as far as the WSAStartup call
is there, which looks to be 7.4.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Jason Erickson wrote:

The only place WSACleanup is being called is libpqdll when the process
detaches the DLL (if the libpq is not staticly linked in), which matches up
with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does not have a
matching WSACleanup. WSACleanup could possibly be placed in freePGconn(),
but unsure if all possible error cases will go through this function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq interface for
Windows.

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

Actually ... why do we need a WSAStartup in makeEmptyPGconn? If we have
one in DLL attach, isn't that sufficient?

The DLL attach function in libpqdll.c is only linked into the binary
when we are doing a WIN32_CLIENT_ONLY build from one of the *.mak files
with MSVC or BCC. It isn't linked in using the standard Makefile under
MinGW.

Seems the proper solution is to link in libpqdll.c for all Win32 builds,
and remove the WSAStartup() call from fe-connect.c, rather than call it
for every connection request. Of course that is only for CVS HEAD.

However, the addition of WSACleanup() is so minor and hard to test that
I would rather do it after 8.0.3 is packaged.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Magnus Hagander wrote:

The only place WSACleanup is being called is libpqdll when

the process

detaches the DLL (if the libpq is not staticly linked in),

which matches up

with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does

not have a

matching WSACleanup. WSACleanup could possibly be placed

in freePGconn(),

but unsure if all possible error cases will go through this

function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq

interface for

Windows.

I agree freePGconn() is the proper place. I will apply the following
patch to current CVS and to 8.0.X. 8.0.3 is being packaged now so I
will wait for 8.0.4.

Actually ... why do we need a WSAStartup in makeEmptyPGconn?
If we have
one in DLL attach, isn't that sufficient?

Not if you link libpq as a static lib...

I *think* that's why it was added. In the beginning it was only in the
DLL attach code, and if you were using the library as a static lib you
had to call it yourself. Not sure when that changed, but clearly that is
when the WSACleanup() call should've gone in.

OK, good. I will document that and have the fix in 8.0.4.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom Lane wrote:

"Magnus Hagander" <mha@sollentuna.net> writes:

Actually ... why do we need a WSAStartup in makeEmptyPGconn?
If we have one in DLL attach, isn't that sufficient?

Not if you link libpq as a static lib...

I *think* that's why it was added. In the beginning it was only in the
DLL attach code, and if you were using the library as a static lib you
had to call it yourself. Not sure when that changed, but clearly that is
when the WSACleanup() call should've gone in.

Ah. Makes sense. So adding it to freePGconn is correct.

Bruce, you should probably back-patch for as far as the WSAStartup call
is there, which looks to be 7.4.

OK, going ahead. I am not thrilled to be fixing this so close to the
release for untestable code.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

This has now been packpatched to 8.0.3 and 7.4.X and will be released in
the next few days.

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

Jason Erickson wrote:

The following bug has been logged online:

Bug reference: 1545
Logged by: Jason Erickson
Email address: jerickso@indian.com
PostgreSQL version: 8.0.1,7.4.7
Operating system: Windows
Description: LIBPQ Windows Version not calling WSACleanup for every
WSAStartup
Details:

Taken from microsoft at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winsock/win
sock/wsastartup_2.asp
-------------------------------------------------------
An application must call one WSACleanup call for every successful WSAStartup
call to allow third-party DLLs to make use of a WS2_32.DLL on behalf of an
application. This means, for example, that if an application calls
WSAStartup three times, it must call WSACleanup three times. The first two
calls to WSACleanup do nothing except decrement an internal counter; the
final WSACleanup call for the task does all necessary resource deallocation
for the task.
------------------------------------------------------------------

The only place WSACleanup is being called is libpqdll when the process
detaches the DLL (if the libpq is not staticly linked in), which matches up
with the WSAStartup when the process attaches to the DLL.

The WSAStartup in the fe-connect.c->makeEmptyPGconn() does not have a
matching WSACleanup. WSACleanup could possibly be placed in freePGconn(),
but unsure if all possible error cases will go through this function.

This problem exists in both 8.0.1 and 7.4.7 of the libpq interface for
Windows.

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073