On Thu, Mar 08, 2007 at 10:37:11AM -0500, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > On Thu, Mar 08, 2007 at 10:10:28AM -0500, Tom Lane wrote:
> >> Also, isn't the WSACleanup() in freePGconn in the wrong place? Seems
> >> like it shouldn't be done until after we've closed the socket. I'd
> >> be inclined to put it at the bottom of the routine.
>
> > Certainly looks wrong. It's interesting how this could have worked
> > *before*.
>
> Because the calls in DllMain covered us (ie, the WSA usage count never
> got to be less than one). If we remove them, we'd better get this pair
> right.
But those calls weren't even compiled in when building using mingw,
which is what the majority of our users have been using lately, I think.
(Since that's what we ship in the binary package)
> One thing that bothers me a bit is that if we just move the call to the
> bottom, then freePGconn won't do it at all if passed a NULL pointer.
> Now (assuming a non-broken app) the only way that can happen is if
> makeEmptyPGconn runs out of memory. If the client gets back a null
> pointer from a connection attempt, it's probably a 50-50 proposition
> whether it will think it ought to do PQfinish with it. So it'd be good
> if we could keep the usage count straight either way. I propose the
> invariant "a WSA usage count is associated with a non-null PGconn
> structure". That would mean that doing WSACleanup only at the bottom
> of freePGconn is correct, but also that makeEmptyPGconn needs to do
> WSACleanup in its (two) failure paths.
I'm honestly unsure wether we need to bother with it, but yeah, that
will likely be "more correct".
(Except one of the error paths in makeEmptyPGconn is already covered,
since it calls freePGconn, which does the WSACleanup)
//Magnus
