Re: Win32 Thread safetyness

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

On 2005-08-24, "Dave Page" <dpage(at)vale-housing(dot)co(dot)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

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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073