Win32 Thread safetyness

On 2005-08-24, "Dave Page" <dpage@vale-housing.co.uk> wrote:

Which relates to:

static unsigned long
pq_threadidcallback(void)
{
return (unsigned long) pthread_self();
}

This is an abuse of pthread_t - it is explicitly not guaranteed in the spec
that pthread_t is an integer type, or even a scalar type; it's permitted to
be a structure. The only valid operations on pthread_t values are assignment
and passing to functions (including pthread_equal() for equality comparison).

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

Unfortunately I just found that we still cannot build in
thread safety mode on Windows, due to an error on my part -
specifically, I concentrated on libpq, not realising that
ecpglib is also thread aware.

It seems that ecpglib uses far more of pthreads than libpq
does, so our mini implementation used in libpq just won't cut
it. I've bitten the bullet (well, more of a jelly bean
actually) and started rewriting things to use the official
win32 pthreads library, however I ran into an error that I'm
not sure about:

Yuck. This sucks :-( I was very much hoping we could avoid an other
build *and* runtime dependency. Which will be a cascading runtime
dependency to each and every program that uses libpq. double-:-(

Anyway, one other concern: Do we *know* how this will interact with
win32 native threads? Meaning will a libpq built against the pthreads
library be safe for *native threads*. Because you can definitly expect
most of the win32 apps that need thread-safeness to be usign native
threads - I've so far not come across a single program that's native
win32 that uses pthreads, whereas almost every program written uses
native threads (though most often not in a way that would need a
threadsafe libpq)

//Magnus

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

Yuck. This sucks :-( I was very much hoping we could avoid an other
build *and* runtime dependency. Which will be a cascading runtime
dependency to each and every program that uses libpq. double-:-(

That seems like a clear nonstarter :-(

Can we confine the damage to stuff that uses ecpg, rather than adding a
dependency to everything that uses libpq?

regards, tom lane

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: 25 August 2005 16:35
To: Magnus Hagander
Cc: Dave Page; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Win32 Thread safetyness

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

Yuck. This sucks :-( I was very much hoping we could avoid an other
build *and* runtime dependency. Which will be a cascading runtime
dependency to each and every program that uses libpq. double-:-(

That seems like a clear nonstarter :-(

Can we confine the damage to stuff that uses ecpg, rather
than adding a
dependency to everything that uses libpq?

Yeah, that could be done with a little makefile hacking - we can use our
emulation for libpq and normal pthreads for ecpglib.

However, I'm still unsure how to handle the problem I posted about. From
what Andrew Supernews has posted about pthread_t not being guaranteed to
be an int, the appropriate fix would be to stop casting it to long. I
haven't looked at the implications of that yet though - any thoughts
before I do?

Regards, Dave.

I poked around this one. The SSL locking documentation is at:

http://www.die.net/doc/linux/man/man3/crypto_set_id_callback.3.html

The description isn't clear. When they talk about locking_function()
and id_function(void), they are talking about arguments to the SSL
functions listed above, not actual SSL functions.

The basic issue with SSL is that it wants some unique identifier for
threads. They really should have defined the function to take pthread_t
rather than unsigned long because pthreads doesn't really give us a
useful way to get an unsigned long value, but we have no choice, so we
have to do it. (I have added a comment to libpq code mentioning the
reason for the cast.) An additional item in the manual is:

id_function(void) is a function that returns a thread ID. It is not
needed on Windows nor on platforms where getpid() returns a different ID
for each thread (most notably Linux).

So, we might be able to get away without this on Win32, or perhaps the
pthread_self().p is a valid unique identifier for Win32 threads and
pthreads.

How is this pthread_self() call working on Win32 now? Is pthreads a
requirement for libpq threading on Win32? I didn't think it was.

As far as ecpg is concerned, I think the right plan is to use Win32
threads for libpq, but to use pthreads in ecpg. However, will Win32
threads still work in ecpg if we use pthreads to do the locking? Maybe
we have to force ecpg users to use pthreads on Win32.

Also, there doesn't seem to be a good way for users to know if libpq or
ecpg was compiled to be thread-safe.

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

Dave Page wrote:

Unfortunately I just found that we still cannot build in thread safety
mode on Windows, due to an error on my part - specifically, I
concentrated on libpq, not realising that ecpglib is also thread aware.

It seems that ecpglib uses far more of pthreads than libpq does, so our
mini implementation used in libpq just won't cut it. I've bitten the
bullet (well, more of a jelly bean actually) and started rewriting
things to use the official win32 pthreads library, however I ran into an
error that I'm not sure about:

make[3]: Entering directory `/cvs/pgsql/src/interfaces/libpq'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wold-style-definition -Wendif-labels
-fno-strict-aliasing -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -DFRONTEND -I. -I../../../src/include
-I./src/include/port/win32 -DEXEC_BACKEND
"-I../../../src/include/port/win32" -I../../../src/port -c -o
fe-secure.o fe-secure.c
fe-secure.c: In function `pq_threadidcallback':
fe-secure.c:879: error: aggregate value used where an integer was
expected

Which relates to:

static unsigned long
pq_threadidcallback(void)
{
return (unsigned long) pthread_self();
}

In pthread.h we have:

typedef struct {
void * p; /* Pointer to actual object */
unsigned int x; /* Extra information - reuse count etc
*/
} ptw32_handle_t;

typedef ptw32_handle_t pthread_t;

PTW32_DLLPORT pthread_t PTW32_CDECL pthread_self (void);

Is it enough just to pass p back on Windows? - eg:

static unsigned long
pq_threadidcallback(void)
{
#ifdef WIN32
return (unsigned long) pthread_self().p;
#else
return (unsigned long) pthread_self();
#endif
}

Everything builds OK with this change - I'm just not sure if it's right.

Regards, Dave

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

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

The basic issue with SSL is that it wants some unique
identifier for threads. They really should have defined the
function to take pthread_t rather than unsigned long because
pthreads doesn't really give us a useful way to get an

They absolutely should not have done that. That would've locked them to
pthreads and not other type of thread implementation.

How is this pthread_self() call working on Win32 now? Is
pthreads a requirement for libpq threading on Win32? I
didn't think it was.

The version that's in CVS now uses our own pthreads wrapper, so it's
definitly not a requirement:
http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/pt
hread-win32.c?rev=1.8

Note that we are not safe per these requirements - pthread_self()
returns GetCurrentThread(), which in turn returns a *pseudo handle that
has the same value in every thread*.
We need to be using GetCurrentThreadId() (it's a DWORD
GetCurrentThreadId(void), so it should be a simple replacement in the
file). Please make that change (I don't think you need a patch for this,
right?) regardless, that's a clear bug in our current implementation.
Should be backpatched as well. (No I haven't tested it, so pleas emake
sure it compiles :P) If you need a patch for it let me know.

As far as ecpg is concerned, I think the right plan is to use
Win32 threads for libpq, but to use pthreads in ecpg.
However, will Win32 threads still work in ecpg if we use
pthreads to do the locking? Maybe we have to force ecpg
users to use pthreads on Win32.

If it works, that's definitly the best. Forcing pthreads on every user
will be almost equal to saying we're not thread-safe on win32. While it
would still be bad for ecpg users, there aren't as many of those as
there are libpq users :-)

Also, there doesn't seem to be a good way for users to know
if libpq or ecpg was compiled to be thread-safe.

Right. A runtime function for this might be a good thing? Like "bool
PQisThreadSafe()" or such?

//Magnus

Magnus Hagander wrote:

The basic issue with SSL is that it wants some unique
identifier for threads. They really should have defined the
function to take pthread_t rather than unsigned long because
pthreads doesn't really give us a useful way to get an

They absolutely should not have done that. That would've locked them to
pthreads and not other type of thread implementation.

True.

How is this pthread_self() call working on Win32 now? Is
pthreads a requirement for libpq threading on Win32? I
didn't think it was.

The version that's in CVS now uses our own pthreads wrapper, so it's
definitly not a requirement:
http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/pt
hread-win32.c?rev=1.8

Note that we are not safe per these requirements - pthread_self()
returns GetCurrentThread(), which in turn returns a *pseudo handle that
has the same value in every thread*.

Ewe.

We need to be using GetCurrentThreadId() (it's a DWORD
GetCurrentThreadId(void), so it should be a simple replacement in the
file). Please make that change (I don't think you need a patch for this,
right?) regardless, that's a clear bug in our current implementation.
Should be backpatched as well. (No I haven't tested it, so pleas emake
sure it compiles :P) If you need a patch for it let me know.

Change made. Thanks. I didn't check the compile but I am sure someone
will and report back a failure. :-)

As far as ecpg is concerned, I think the right plan is to use
Win32 threads for libpq, but to use pthreads in ecpg.
However, will Win32 threads still work in ecpg if we use
pthreads to do the locking? Maybe we have to force ecpg
users to use pthreads on Win32.

If it works, that's definitly the best. Forcing pthreads on every user
will be almost equal to saying we're not thread-safe on win32. While it
would still be bad for ecpg users, there aren't as many of those as
there are libpq users :-)

Also, there doesn't seem to be a good way for users to know
if libpq or ecpg was compiled to be thread-safe.

Right. A runtime function for this might be a good thing? Like "bool
PQisThreadSafe()" or such?

Yes, and a flag to ecpg. Added to TODO:

* Add function to return the thread safety status of libpq and ecpg

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

-----Original Message-----
From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
Sent: 28 August 2005 18:19
To: Dave Page
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Win32 Thread safetyness

I poked around this one. The SSL locking documentation is at:

http://www.die.net/doc/linux/man/man3/crypto_set_id_callback.3.html

The description isn't clear. When they talk about locking_function()
and id_function(void), they are talking about arguments to the SSL
functions listed above, not actual SSL functions.

The basic issue with SSL is that it wants some unique identifier for
threads. They really should have defined the function to
take pthread_t
rather than unsigned long because pthreads doesn't really give us a
useful way to get an unsigned long value, but we have no choice, so we
have to do it. (I have added a comment to libpq code mentioning the
reason for the cast.) An additional item in the manual is:

id_function(void) is a function that returns a thread
ID. It is not
needed on Windows nor on platforms where getpid()
returns a different ID
for each thread (most notably Linux).

So, we might be able to get away without this on Win32, or perhaps the
pthread_self().p is a valid unique identifier for Win32 threads and
pthreads.

Right.

How is this pthread_self() call working on Win32 now? Is pthreads a
requirement for libpq threading on Win32? I didn't think it was.

Libpq can use our existing minimal implementation of pthreads (this is
how it currently works in CVS). The thread test program needs the full
pthreads library.

As far as ecpg is concerned, I think the right plan is to use Win32
threads for libpq, but to use pthreads in ecpg. However, will Win32
threads still work in ecpg if we use pthreads to do the
locking? Maybe
we have to force ecpg users to use pthreads on Win32.

Currently I have it building using the full pthreads library, with

static unsigned long
pq_threadidcallback(void)
{
#ifdef WIN32
return (unsigned long) pthread_self().p;
#else
return (unsigned long) pthread_self();
#endif
}

I don't know how badly that might be screwed up though - I simply don't
know enough about pthreads (or win32 threads for that matter - I only
really use wxWidgets threads or C# threads).

The ideal way would be to port it to win32 threads of course, but
there's no way I'll have time to do that at the moment with pgAdmin and
pgInstaller both needing my attention :-(. Anyone else got some time?

Also, there doesn't seem to be a good way for users to know
if libpq or
ecpg was compiled to be thread-safe.

No. Mind you, that ties in nicely with a comment that Magnus made to me
the other day - we could really use a function to get libpq's version
number. Something similar (for 8.2 of course) could tell us more about
it, such as thread safetyness.

Anyhoo, I can patch ecpglib to use the full pthreads with my hack above
if people think that is the way to go for the time being.

Regards, Dave.

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

Magnus Hagander wrote:

Also, there doesn't seem to be a good way for users to know
if libpq or ecpg was compiled to be thread-safe.

Right. A runtime function for this might be a good thing? Like "bool
PQisThreadSafe()" or such?

Yes, and a flag to ecpg. Added to TODO:

Um, it's not clear *when* you need to know this:
- application configure time?
- application compile time?
- application link time?
- application run time?

Of those possibilities, "add a function" responds to only one, and it's
the one I can see least use-case for. I should think that by run-time
it's probably too late to do much about it other than fail.

You can find out whether thread-safety was mentioned in our configure
settings by looking at the result of pg_config --configure. This might
be enough for the find-out-at-configure-time case.

regards, tom lane

Dave Page wrote:

So, we might be able to get away without this on Win32, or perhaps the
pthread_self().p is a valid unique identifier for Win32 threads and
pthreads.

Right.

How is this pthread_self() call working on Win32 now? Is pthreads a
requirement for libpq threading on Win32? I didn't think it was.

Libpq can use our existing minimal implementation of pthreads (this is
how it currently works in CVS). The thread test program needs the full
pthreads library.

Well, actually I tried to run configure --enable-thread-safety on a
MinGW platform and got:

configure: error: pthread.h not found, required for --enable-thread-safety
You need to run the 'configure' program first. See the file
'INSTALL' for installation instructions.
make: *** [all] Error 1

Can we not compile --enable-thread-safety without pthreads on Win32? I
am thinking that is true because we are going to need pthreads for ecpg.

As far as ecpg is concerned, I think the right plan is to use Win32
threads for libpq, but to use pthreads in ecpg. However, will Win32
threads still work in ecpg if we use pthreads to do the
locking? Maybe
we have to force ecpg users to use pthreads on Win32.

Currently I have it building using the full pthreads library, with

static unsigned long
pq_threadidcallback(void)
{
#ifdef WIN32
return (unsigned long) pthread_self().p;
#else
return (unsigned long) pthread_self();
#endif
}

I don't know how badly that might be screwed up though - I simply don't
know enough about pthreads (or win32 threads for that matter - I only
really use wxWidgets threads or C# threads).

Seems Mangus gave us the proper Win32 function call and I have changed
pthread_self() on Win32 to return DWORD.

The ideal way would be to port it to win32 threads of course, but
there's no way I'll have time to do that at the moment with pgAdmin and
pgInstaller both needing my attention :-(. Anyone else got some time?

Also, there doesn't seem to be a good way for users to know
if libpq or
ecpg was compiled to be thread-safe.

No. Mind you, that ties in nicely with a comment that Magnus made to me
the other day - we could really use a function to get libpq's version
number. Something similar (for 8.2 of course) could tell us more about
it, such as thread safetyness.

True.

Anyhoo, I can patch ecpglib to use the full pthreads with my hack above
if people think that is the way to go for the time being.

OK.

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

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

Magnus Hagander wrote:

Also, there doesn't seem to be a good way for users to know
if libpq or ecpg was compiled to be thread-safe.

Right. A runtime function for this might be a good thing? Like "bool
PQisThreadSafe()" or such?

Yes, and a flag to ecpg. Added to TODO:

Um, it's not clear *when* you need to know this:
- application configure time?
- application compile time?
- application link time?
- application run time?

Of those possibilities, "add a function" responds to only one, and it's
the one I can see least use-case for. I should think that by run-time
it's probably too late to do much about it other than fail.

Yea, I am thinking a libpq function would say "libpq not thread-safe'
and return an error. I would think pg_config output would be fine for
any link or compile-time check.

You can find out whether thread-safety was mentioned in our configure
settings by looking at the result of pg_config --configure. This might
be enough for the find-out-at-configure-time case.

True.

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

-----Original Message-----
From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
Sent: 28 August 2005 22:56
To: Dave Page
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Win32 Thread safetyness

Well, actually I tried to run configure --enable-thread-safety on a
MinGW platform and got:

configure: error: pthread.h not found, required for
--enable-thread-safety
You need to run the 'configure' program first. See the file
'INSTALL' for installation instructions.
make: *** [all] Error 1

Can we not compile --enable-thread-safety without pthreads on
Win32? I
am thinking that is true because we are going to need
pthreads for ecpg.

Correct - it is needed for ecpg, but not libpq.

Anyhoo, I can patch ecpglib to use the full pthreads with

my hack above

if people think that is the way to go for the time being.

OK.

Attached (without the hack). Libpq will use it's minimal pthread
implementation, ecpg will use pthreadGC2.

You'll need to run autoconf after applying.

Regards, Dave.

Patch applied. Autoconf run. Thanks.

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

Dave Page wrote:

-----Original Message-----
From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
Sent: 28 August 2005 22:56
To: Dave Page
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Win32 Thread safetyness

Well, actually I tried to run configure --enable-thread-safety on a
MinGW platform and got:

configure: error: pthread.h not found, required for
--enable-thread-safety
You need to run the 'configure' program first. See the file
'INSTALL' for installation instructions.
make: *** [all] Error 1

Can we not compile --enable-thread-safety without pthreads on
Win32? I
am thinking that is true because we are going to need
pthreads for ecpg.

Correct - it is needed for ecpg, but not libpq.

Anyhoo, I can patch ecpglib to use the full pthreads with

my hack above

if people think that is the way to go for the time being.

OK.

Attached (without the hack). Libpq will use it's minimal pthread
implementation, ecpg will use pthreadGC2.

You'll need to run autoconf after applying.

Regards, Dave.

Content-Description: win32-threads.diff

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: 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

Yes, and a flag to ecpg. Added to TODO:

Um, it's not clear *when* you need to know this:
- application configure time?
- application compile time?
- application link time?
- application run time?

Of those possibilities, "add a function" responds to only one, and
it's the one I can see least use-case for. I should think that by
run-time it's probably too late to do much about it other than fail.

Yea, I am thinking a libpq function would say "libpq not thread-safe'
and return an error. I would think pg_config output would be
fine for any link or compile-time check.

Another idea might be to compile the library with a different name when
it's thread safe. And if your system supports it, we build both by
default. (libpq and libpqt or something).

//Magnus