Re: --enable-thread-safety on Win32

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailtogsql-hackers-owner@postgresql.org] On Behalf Of Dave Page
> Sent: 28 July 2005 16:16
> To: Bruce Momjian; Tom Lane
> Cc: PostgreSQL-development
> Subject: Re: [HACKERS] --enable-thread-safety on Win32
>
>
> > OK, but I would then like someone to actually run the tests we do in
> > thread_test.c and make sure they _would_ pass on Win32.
>
> OK, I will work on this, and subsequently fixing configure etc.

OK, I have the thread test working with the fully pthreads library on
Windows, and everything passes except errno (well, and getpwuid which we
don't have anyway). It is supposed to be thread safe when apps are
either built against libcmt.lib or msvcrt.dll (which we use), however it
still seems to fail on Mingw. From what I can find, the 'usual' way to
make errno thread safe is by using _beginthreadex() instead of
CreateThread(), but that is done by the application of course, not libpq
(in the test case, it would be done by pthreads).

See http://www.microsoft.com/msj/0799/Win32/Win320799.aspx for a
discussion of this if interested.

However.... In all but one place in libpq, we don't use errno anyway
(actually 2, but one is a bug anyway) because we use GetLastError()
instead (which tested thread safe as well FWIW). The only place it's
used is PQoidValue():

result = strtoul(res->cmdStatus + 7, &endptr, 10);

if (!endptr || (*endptr != ' ' && *endptr != '\0') || errno ==
ERANGE)
return InvalidOid;
else
return (Oid) result;

We don't believe strtoul() works with GetLastError() unfortunately. One
(hackish) solution would be to check that it doesn't return 0 or
ULONG_MAX.

Any suggestions?

Aside from this issue, the hacked test app, and the changes to make all
this compile are done.

Regards, Dave

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

"Dave Page" <dpage@vale-housing.co.uk> writes:
> However.... In all but one place in libpq, we don't use errno anyway
> (actually 2, but one is a bug anyway) because we use GetLastError()
> instead (which tested thread safe as well FWIW). The only place it's
> used is PQoidValue():

> result = strtoul(res->cmdStatus + 7, &endptr, 10);

> if (!endptr || (*endptr != ' ' && *endptr != '\0') || errno ==
> ERANGE)
> return InvalidOid;
> else
> return (Oid) result;

> We don't believe strtoul() works with GetLastError() unfortunately. One
> (hackish) solution would be to check that it doesn't return 0 or
> ULONG_MAX.

I'm not sure why we bother with an overflow check there at all. It'd be
worth checking that there is a digit at cmdStatus + 7, but as long as
there is one, it's difficult to see why an overflow check is needed.

The only justification that comes to mind is that if someday there are
versions of Postgres that have 64-bit OIDs, you could get an overflow
here if you had a 32-bit-OID libpq talking to a 64-bit server. However,
I don't see a particularly good reason to return InvalidOid instead of
an overflowed value anyway in that situation. For PQoidValue,
InvalidOid is supposed to mean "there is no OID in this command status"
not "there is an OID but I cannot represent it".

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Tom Lane wrote:
> "Dave Page" <dpage@vale-housing.co.uk> writes:
> > However.... In all but one place in libpq, we don't use errno anyway
> > (actually 2, but one is a bug anyway) because we use GetLastError()
> > instead (which tested thread safe as well FWIW). The only place it's
> > used is PQoidValue():
>
> > result = strtoul(res->cmdStatus + 7, &endptr, 10);
>
> > if (!endptr || (*endptr != ' ' && *endptr != '\0') || errno ==
> > ERANGE)
> > return InvalidOid;
> > else
> > return (Oid) result;
>
> > We don't believe strtoul() works with GetLastError() unfortunately. One
> > (hackish) solution would be to check that it doesn't return 0 or
> > ULONG_MAX.
>
> I'm not sure why we bother with an overflow check there at all. It'd be
> worth checking that there is a digit at cmdStatus + 7, but as long as
> there is one, it's difficult to see why an overflow check is needed.
>
> The only justification that comes to mind is that if someday there are
> versions of Postgres that have 64-bit OIDs, you could get an overflow
> here if you had a 32-bit-OID libpq talking to a 64-bit server. However,
> I don't see a particularly good reason to return InvalidOid instead of
> an overflowed value anyway in that situation. For PQoidValue,
> InvalidOid is supposed to mean "there is no OID in this command status"
> not "there is an OID but I cannot represent it".

I disabled the check on Win32, and added a comment explaining why. We
could disable it just when we use thread-safety, but changing the
behavior for threading didn't seems wise.

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

Index: src/interfaces/libpq/fe-exec.c
================================================== =================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.170
diff -c -c -r1.170 fe-exec.c
*** src/interfaces/libpq/fe-exec.c 2 Jul 2005 17:01:54 -0000 1.170
--- src/interfaces/libpq/fe-exec.c 12 Aug 2005 23:58:16 -0000
***************
*** 2166,2172 ****
#endif
result = strtoul(res->cmdStatus + 7, &endptr, 10);

! if (!endptr || (*endptr != ' ' && *endptr != '\0') || errno == ERANGE)
return InvalidOid;
else
return (Oid) result;
--- 2166,2180 ----
#endif
result = strtoul(res->cmdStatus + 7, &endptr, 10);

! if (!endptr || (*endptr != ' ' && *endptr != '\0')
! #ifndef WIN32
! /*
! * On WIN32, errno is not thread-safe and GetLastError() isn't set by
! * strtoul(), so we can't check on this platform.
! */
! || errno == ERANGE
! #endif
! )
return InvalidOid;
else
return (Oid) result;


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq