Re: Win32 Thread safetyness

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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(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