Re: Compiling libpq with VisualC

Some modifications are needed to compile libpq with vc6:

fe-connect.c:
- pg_config_paths.h isn't available. SYSCONFDIR is already defined so
fe-connect.c doesn't need to include that.
patch appended

win32.mak:
- pg_config.h must be generated
- port/pgstrcasecmp.c is needed
- port/path.c is not needed
- the current cvs version includes some bogus cr characters, when
checking out with wincvs the file will contain several cr cr lf lines
which will confuse nmake.

Because the last problem might be difficult to handle as diff, I
attached win32.mak as complete file, with all cr stripped so it may be
committed safely with any cvs client.

Regards,
Andreas

I noticed that SYSCONFDIR will also be an issue for relocatable installs
just like localedir. I will address them in the same way either as an
environment variable or libpq function call to set the location.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

Andreas Pflug wrote:
> Some modifications are needed to compile libpq with vc6:
>
> fe-connect.c:
> - pg_config_paths.h isn't available. SYSCONFDIR is already defined so
> fe-connect.c doesn't need to include that.
> patch appended
>
> win32.mak:
> - pg_config.h must be generated
> - port/pgstrcasecmp.c is needed
> - port/path.c is not needed
> - the current cvs version includes some bogus cr characters, when
> checking out with wincvs the file will contain several cr cr lf lines
> which will confuse nmake.
>
> Because the last problem might be difficult to handle as diff, I
> attached win32.mak as complete file, with all cr stripped so it may be
> committed safely with any cvs client.
>
> Regards,
> Andreas

> Index: fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.271
> diff -u -r1.271 fe-connect.c
> --- fe-connect.c 26 May 2004 18:35:51 -0000 1.271
> +++ fe-connect.c 28 May 2004 11:48:56 -0000
> @@ -29,7 +29,10 @@
> #include "libpq-fe.h"
> #include "libpq-int.h"
> #include "fe-auth.h"
> +
> +#ifndef SYSCONFDIR
> #include "pg_config_paths.h"
> +#endif
>
> #ifdef WIN32
> #include "win32.h"
>

> # Makefile for Microsoft Visual C++ 5.0 (or compat)
>
> # Will build a Win32 static library libpq(d).lib
> # and a Win32 dynamic library libpq(d).dll with import library libpq(d)dll.lib
> # USE_SSL=1 will compile with OpenSSL
> # DEBUG=1 compiles with debugging symbols
>
>
> !MESSAGE Building the Win32 static library...
> !MESSAGE
>
> !IFDEF DEBUG
> OPT=/Od /Zi /MDd
> LOPT=/DEBUG
> DEBUGDEF=/D _DEBUG
> OUTFILENAME=libpqd
> !ELSE
> OPT=/O2 /MD
> LOPT=
> DEBUGDEF=/D NDEBUG
> OUTFILENAME=libpq
> !ENDIF
>
> !IF "$(OS)" == "Windows_NT"
> NULL=
> !ELSE
> NULL=nul
> !ENDIF
>
> CPP=cl.exe
> RSC=rc.exe
>
> !IFDEF DEBUG
> OUTDIR=.\Debug
> INTDIR=.\Debug
> CPP_OBJS=.\Debug/
> !ELSE
> OUTDIR=.\Release
> INTDIR=.\Release
> CPP_OBJS=.\Release/
> !ENDIF
>
>
> ALL : config "$(OUTDIR)\$(OUTFILENAME).lib" "$(OUTDIR)\$(OUTFILENAME).dll"
>
> CLEAN :
> -(at)erase "$(INTDIR)\getaddrinfo.obj"
> -(at)erase "$(INTDIR)\pgstrcasecmp.obj"
> -(at)erase "$(INTDIR)\thread.obj"
> -(at)erase "$(INTDIR)\inet_aton.obj"
> -(at)erase "$(INTDIR)\crypt.obj"
> -(at)erase "$(INTDIR)\noblock.obj"
> -(at)erase "$(INTDIR)\dllist.obj"
> -(at)erase "$(INTDIR)\md5.obj"
> -(at)erase "$(INTDIR)\ip.obj"
> -(at)erase "$(INTDIR)\fe-auth.obj"
> -(at)erase "$(INTDIR)\fe-protocol2.obj"
> -(at)erase "$(INTDIR)\fe-protocol3.obj"
> -(at)erase "$(INTDIR)\fe-connect.obj"
> -(at)erase "$(INTDIR)\fe-exec.obj"
> -(at)erase "$(INTDIR)\fe-lobj.obj"
> -(at)erase "$(INTDIR)\fe-misc.obj"
> -(at)erase "$(INTDIR)\fe-print.obj"
> -(at)erase "$(INTDIR)\fe-secure.obj"
> -(at)erase "$(INTDIR)\pqexpbuffer.obj"
> -(at)erase "$(OUTDIR)\libpqdll.obj"
> -(at)erase "$(OUTDIR)\win32.obj"
> -(at)erase "$(OUTDIR)\$(OUTFILENAME).lib"
> -(at)erase "$(OUTDIR)\$(OUTFILENAME).dll"
> -(at)erase "$(OUTDIR)\libpq.res"
> -(at)erase "*.pch"
> -(at)erase "$(OUTDIR)\libpq.pch"
> -(at)erase "$(OUTDIR)\$(OUTFILENAME)dll.exp"
> -(at)erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
> -(at)erase "$(INTDIR)\wchar.obj"
> -(at)erase "$(INTDIR)\encnames.obj"
>
>
>
> config: ..\..\include\pg_config.h
>
> ..\..\include\pg_config.h: ..\..\include\pg_config.h.win32
> copy ..\..\include\pg_config.h.win32 ..\..\include\pg_config.h
>
>
> "$(OUTDIR)" :
> if not exist "$(OUTDIR)/$(NULL)" mkdir "$(OUTDIR)"
>
> CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /D "FRONTEND" $(DEBUGDEF) /D\
> "WIN32" /D "_WINDOWS" /Fp"$(INTDIR)\libpq.pch" /YX\
> /Fo"$(INTDIR)\\" /Fd"$(INTDIR)\\" /FD /c /D "HAVE_VSNPRINTF" /D "HAVE_STRDUP"
>
> !IFDEF USE_SSL
> CPP_PROJ=$(CPP_PROJ) /D USE_SSL
> SSL_LIBS=ssleay32.lib libeay32.lib gdi32.lib
> !ENDIF
>
> CPP_SBRS=.
>
> LIB32=link.exe -lib
> LIB32_FLAGS=$(LOPT) /nologo /out:"$(OUTDIR)\$(OUTFILENAME).lib"
> LIB32_OBJS= \
> "$(INTDIR)\win32.obj" \
> "$(INTDIR)\getaddrinfo.obj" \
> "$(INTDIR)\pgstrcasecmp.obj" \
> "$(INTDIR)\thread.obj" \
> "$(INTDIR)\inet_aton.obj" \
> "$(INTDIR)\crypt.obj" \
> "$(INTDIR)\noblock.obj" \
> "$(INTDIR)\dllist.obj" \
> "$(INTDIR)\md5.obj" \
> "$(INTDIR)\ip.obj" \
> "$(INTDIR)\fe-auth.obj" \
> "$(INTDIR)\fe-protocol2.obj" \
> "$(INTDIR)\fe-protocol3.obj" \
> "$(INTDIR)\fe-connect.obj" \
> "$(INTDIR)\fe-exec.obj" \
> "$(INTDIR)\fe-lobj.obj" \
> "$(INTDIR)\fe-misc.obj" \
> "$(INTDIR)\fe-print.obj" \
> "$(INTDIR)\fe-secure.obj" \
> "$(INTDIR)\pqexpbuffer.obj" \
> "$(INTDIR)\wchar.obj" \
> "$(INTDIR)\encnames.obj"
>
>
> RSC_PROJ=/l 0x409 /fo"$(INTDIR)\libpq.res"
>
> LINK32=link.exe
> LINK32_FLAGS=kernel32.lib user32.lib advapi32.lib wsock32.lib $(SSL_LIBS) \
> /nologo /subsystem:windows /dll $(LOPT) /incremental:no\
> /pdb:"$(OUTDIR)\libpqdll.pdb" /machine:I386 /out:"$(OUTDIR)\$(OUTFILENAME).dll"\
> /implib:"$(OUTDIR)\$(OUTFILENAME)dll.lib" /def:$(OUTFILENAME)dll.def
> LINK32_OBJS= \
> "$(INTDIR)\libpqdll.obj" \
> "$(OUTDIR)\$(OUTFILENAME).lib" \
> "$(OUTDIR)\libpq.res"
>
>
> "$(OUTDIR)\$(OUTFILENAME).lib" : "$(OUTDIR)" $(DEF_FILE) $(LIB32_OBJS)
> $(LIB32) @<<
> $(LIB32_FLAGS) $(DEF_FLAGS) $(LIB32_OBJS)
> <<
>
> "$(INTDIR)\libpq.res" : "$(INTDIR)" libpq.rc
> $(RSC) $(RSC_PROJ) libpq.rc
>
>
> "$(OUTDIR)\$(OUTFILENAME).dll" : "$(OUTDIR)" "$(OUTDIR)\libpqdll.obj" "$(INTDIR)\libpqdll.obj" "$(INTDIR)\libpq.res"
> $(LINK32) @<<
> $(LINK32_FLAGS) $(LINK32_OBJS)
> <<
>
> "$(INTDIR)\getaddrinfo.obj" : ..\..\port\getaddrinfo.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\getaddrinfo.c
> <<
>
> "$(INTDIR)\pgstrcasecmp.obj" : ..\..\port\pgstrcasecmp.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\pgstrcasecmp.c
> <<
>
> "$(INTDIR)\thread.obj" : ..\..\port\thread.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\thread.c
> <<
>
> "$(INTDIR)\inet_aton.obj" : ..\..\port\inet_aton.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\inet_aton.c
> <<
>
> "$(INTDIR)\crypt.obj" : ..\..\port\crypt.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\crypt.c
> <<
>
> "$(INTDIR)\noblock.obj" : ..\..\port\noblock.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\port\noblock.c
> <<
>
> "$(INTDIR)\dllist.obj" : ..\..\backend\lib\dllist.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\backend\lib\dllist.c
> <<
>
>
> "$(INTDIR)\md5.obj" : ..\..\backend\libpq\md5.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\backend\libpq\md5.c
> <<
>
> "$(INTDIR)\ip.obj" : ..\..\backend\libpq\ip.c
> $(CPP) @<<
> $(CPP_PROJ) ..\..\backend\libpq\ip.c
> <<
>
> "$(INTDIR)\wchar.obj" : ..\..\backend\utils\mb\wchar.c
> $(CPP) @<<
> $(CPP_PROJ) /I "." ..\..\backend\utils\mb\wchar.c
> <<
>
>
> "$(INTDIR)\encnames.obj" : ..\..\backend\utils\mb\encnames.c
> $(CPP) @<<
> $(CPP_PROJ) /I "." ..\..\backend\utils\mb\encnames.c
> <<
>
>
> .c{$(CPP_OBJS)}.obj::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<
>
> .cpp{$(CPP_OBJS)}.obj::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<
>
> .cxx{$(CPP_OBJS)}.obj::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<
>
> .c{$(CPP_SBRS)}.sbr::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<
>
> .cpp{$(CPP_SBRS)}.sbr::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<
>
> .cxx{$(CPP_SBRS)}.sbr::
> $(CPP) @<<
> $(CPP_PROJ) $<
> <<

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

--
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 wrote:
>
> I noticed that SYSCONFDIR will also be an issue for relocatable installs
> just like localedir. I will address them in the same way either as an
> environment variable or libpq function call to set the location.

Fixed.

> >
> > fe-connect.c:
> > - pg_config_paths.h isn't available. SYSCONFDIR is already defined so
> > fe-connect.c doesn't need to include that.
> > patch appended

This shouldn't be needed anymore. Where is SYSCONFDIR coming from? Is
it from some Win32 include file? It should only be coming from
pg_config_paths.h now.

> > win32.mak:
> > - pg_config.h must be generated
> > - port/pgstrcasecmp.c is needed
> > - port/path.c is not needed
> > - the current cvs version includes some bogus cr characters, when
> > checking out with wincvs the file will contain several cr cr lf lines
> > which will confuse nmake.
> >
> > Because the last problem might be difficult to handle as diff, I
> > attached win32.mak as complete file, with all cr stripped so it may be
> > committed safely with any cvs client.

Added. Thanks.

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

>
>
>>>fe-connect.c:
>>>- pg_config_paths.h isn't available. SYSCONFDIR is already defined so
>>>fe-connect.c doesn't need to include that.
>>>patch appended
>>>
>>>
>
>This shouldn't be needed anymore. Where is SYSCONFDIR coming from? Is
>it from some Win32 include file? It should only be coming from
>pg_config_paths.h now.
>
>
It's defined in pg_config.h.win32, which is copied to pg_config.h.
There's no pg_config_paths.h for win32, and I wonder if the code which
uses SYSCONFDIR is of any use for win32.

Regards,
Andreas

Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >
> >
> >>>fe-connect.c:
> >>>- pg_config_paths.h isn't available. SYSCONFDIR is already defined so
> >>>fe-connect.c doesn't need to include that.
> >>>patch appended
> >>>
> >>>
> >
> >This shouldn't be needed anymore. Where is SYSCONFDIR coming from? Is
> >it from some Win32 include file? It should only be coming from
> >pg_config_paths.h now.
> >
> >
> It's defined in pg_config.h.win32, which is copied to pg_config.h.
> There's no pg_config_paths.h for win32, and I wonder if the code which
> uses SYSCONFDIR is of any use for win32.

Thanks. Removed. I never saw that because the file has a .win32
extension and my search tools didn't look in there.

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

>Andreas Pflug wrote:
>
>
>>Bruce Momjian wrote:
>>
>>
>>
>>>
>>>
>>>
>>>
>>>>>fe-connect.c:
>>>>>- pg_config_paths.h isn't available. SYSCONFDIR is already defined so
>>>>>fe-connect.c doesn't need to include that.
>>>>>patch appended
>>>>>
>>>>>
>>>>>
>>>>>
>>>This shouldn't be needed anymore. Where is SYSCONFDIR coming from? Is
>>>it from some Win32 include file? It should only be coming from
>>>pg_config_paths.h now.
>>>
>>>
>>>
>>>
>>It's defined in pg_config.h.win32, which is copied to pg_config.h.
>>There's no pg_config_paths.h for win32, and I wonder if the code which
>>uses SYSCONFDIR is of any use for win32.
>>
>>
>
>Thanks. Removed. I never saw that because the file has a .win32
>extension and my search tools didn't look in there.
>
>
Well, that doesn't work like this. As I already wrote, there's no
SYSCONFDIR, and if there was it can't have any sensible value for win32
clients. SYSCONFDIR as of pg_config.h.win32 version 1.13 was just a
dummy to make the compiler happy. Now there's no SYSCONFDIR around, and
no pg_config_paths.h either...

The attached patch will create a dummy pg_config_paths.h. Additionally,
ENABLE_THREAD_SAFETY is supported by the makefile (but not by the
sources, which need some rework)

Regards,
Andreas

Andreas Pflug wrote:

>
> The attached patch will create a dummy pg_config_paths.h.
> Additionally, ENABLE_THREAD_SAFETY is supported by the makefile (but
> not by the sources, which need some rework)
>

Now including the patch...
Regards,
Andreas

Patch applied. I didn't realize that file was for win32 _client_
compiles, and I didn't realize it had no pg_config_paths.h.

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

Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >Andreas Pflug wrote:
> >
> >
> >>Bruce Momjian wrote:
> >>
> >>
> >>
> >>>
> >>>
> >>>
> >>>
> >>>>>fe-connect.c:
> >>>>>- pg_config_paths.h isn't available. SYSCONFDIR is already defined so
> >>>>>fe-connect.c doesn't need to include that.
> >>>>>patch appended
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>This shouldn't be needed anymore. Where is SYSCONFDIR coming from? Is
> >>>it from some Win32 include file? It should only be coming from
> >>>pg_config_paths.h now.
> >>>
> >>>
> >>>
> >>>
> >>It's defined in pg_config.h.win32, which is copied to pg_config.h.
> >>There's no pg_config_paths.h for win32, and I wonder if the code which
> >>uses SYSCONFDIR is of any use for win32.
> >>
> >>
> >
> >Thanks. Removed. I never saw that because the file has a .win32
> >extension and my search tools didn't look in there.
> >
> >
> Well, that doesn't work like this. As I already wrote, there's no
> SYSCONFDIR, and if there was it can't have any sensible value for win32
> clients. SYSCONFDIR as of pg_config.h.win32 version 1.13 was just a
> dummy to make the compiler happy. Now there's no SYSCONFDIR around, and
> no pg_config_paths.h either...
>
> The attached patch will create a dummy pg_config_paths.h. Additionally,
> ENABLE_THREAD_SAFETY is supported by the makefile (but not by the
> sources, which need some rework)
>
> Regards,
> Andreas
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

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

The appended patch implements ENABLE_THREAD_SAFETY for win32 compiled
with Visual C. Two additional files will supply the needed pthread stuff.

There's no SIGPIPE for native win32, so there are some #IFNDEF WIN32 to
skip installing a signal handler for this. I'm not sure if this is going
to show conflicts for CYGWIN or other unix-like builds on win32
platforms, i.e. if WIN32 is defined in such cases or not; please check.

Note: the previously posted win32.mak patch is superseded by this.

Regards,
Andreas

I have looked over this patch. I noticed this:

-static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
-
+ static pthread_mutex_t init_mutex;
+ static int mutex_initialized = 0;
+ if (!mutex_initialized)
+ {
+ mutex_initialized = 1;
+ pthread_mutex_init(&init_mutex, NULL);
+ }

While this might work using your pthread compatibility implementation
using CreateMutex(), it will not work on a native pthread implementation
because you can only call pthread_mutex_init() once. Your code allows
two threads to both call it.

Now, how to fix it? You need some kind of pthread_once() call or
somehow set up a critical section that will prevent two threads from
doing the initialization, but there is no method using your API to
create the critical section without creating a lock first.

I am thinking we need to put your initialization code in #ifdef WIN32
and keep the native pthread code unchanged.

Also, do you not have the problem with SIGPIPE from send(), so you don't
need set/get_thread_specific()?

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

Andreas Pflug wrote:
> The appended patch implements ENABLE_THREAD_SAFETY for win32 compiled
> with Visual C. Two additional files will supply the needed pthread stuff.
>
> There's no SIGPIPE for native win32, so there are some #IFNDEF WIN32 to
> skip installing a signal handler for this. I'm not sure if this is going
> to show conflicts for CYGWIN or other unix-like builds on win32
> platforms, i.e. if WIN32 is defined in such cases or not; please check.
>
> Note: the previously posted win32.mak patch is superseded by this.
>
> Regards,
> Andreas

> Index: win32.mak
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/win32.mak,v
> retrieving revision 1.23
> diff -u -r1.23 win32.mak
> --- win32.mak 3 Jun 2004 00:11:13 -0000 1.23
> +++ win32.mak 4 Jun 2004 13:26:39 -0000
> @@ -74,19 +74,25 @@
> -(at)erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
> -(at)erase "$(INTDIR)\wchar.obj"
> -(at)erase "$(INTDIR)\encnames.obj"
> + -(at)erase "$(INTDIR)\pthread-win32.obj"
>
>
>
> -config: ..\..\include\pg_config.h
> +config: ..\..\include\pg_config.h pthread.h pg_config_paths.h
>
> ..\..\include\pg_config.h: ..\..\include\pg_config.h.win32
> copy ..\..\include\pg_config.h.win32 ..\..\include\pg_config.h
>
> +pthread.h: pthread.h.win32
> + copy pthread.h.win32 pthread.h
> +
> +pg_config_paths.h: win32.mak
> + echo #define SYSCONFDIR "" >pg_config_paths.h
>
> "$(OUTDIR)" :
> if not exist "$(OUTDIR)/$(NULL)" mkdir "$(OUTDIR)"
>
> -CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /D "FRONTEND" $(DEBUGDEF) /D\
> +CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /I. /D "FRONTEND" $(DEBUGDEF) /D\
> "WIN32" /D "_WINDOWS" /Fp"$(INTDIR)\libpq.pch" /YX\
> /Fo"$(INTDIR)\\" /Fd"$(INTDIR)\\" /FD /c /D "HAVE_VSNPRINTF" /D "HAVE_STRDUP"
>
> @@ -95,6 +101,10 @@
> SSL_LIBS=ssleay32.lib libeay32.lib gdi32.lib
> !ENDIF
>
> +!IFDEF ENABLE_THREAD_SAFETY
> +CPP_PROJ=$(CPP_PROJ) /D ENABLE_THREAD_SAFETY
> +!ENDIF
> +
> CPP_SBRS=.
>
> LIB32=link.exe -lib
> @@ -121,7 +131,8 @@
> "$(INTDIR)\fe-secure.obj" \
> "$(INTDIR)\pqexpbuffer.obj" \
> "$(INTDIR)\wchar.obj" \
> - "$(INTDIR)\encnames.obj"
> + "$(INTDIR)\encnames.obj" \
> + "$(INTDIR)\pthread-win32.obj"
>
>
> RSC_PROJ=/l 0x409 /fo"$(INTDIR)\libpq.res"
> Index: fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.272
> diff -u -r1.272 fe-connect.c
> --- fe-connect.c 3 Jun 2004 00:07:38 -0000 1.272
> +++ fe-connect.c 4 Jun 2004 13:26:43 -0000
> @@ -882,11 +882,13 @@
> const char *node = NULL;
> int ret;
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
>
> /* Check only on first connection request */
> pthread_once(&check_sigpipe_once, check_sigpipe_handler);
> #endif
> +#endif
>
> if (!conn)
> return 0;
> @@ -3187,7 +3189,13 @@
> default_threadlock(int acquire)
> {
> #ifdef ENABLE_THREAD_SAFETY
> - static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
> + static pthread_mutex_t singlethread_lock;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&singlethread_lock, NULL);
> + }
> if (acquire)
> pthread_mutex_lock(&singlethread_lock);
> else
> Index: fe-secure.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
> retrieving revision 1.41
> diff -u -r1.41 fe-secure.c
> --- fe-secure.c 3 Jun 2004 00:13:19 -0000 1.41
> +++ fe-secure.c 4 Jun 2004 13:26:45 -0000
> @@ -864,8 +864,13 @@
> init_ssl_system(PGconn *conn)
> {
> #ifdef ENABLE_THREAD_SAFETY
> -static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
> -
> + static pthread_mutex_t init_mutex;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&init_mutex, NULL);
> + }
> pthread_mutex_lock(&init_mutex);
>
> if (pq_initssllib && pq_lockarray == NULL) {
> @@ -1171,6 +1176,7 @@
>
>
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> /*
> * Check SIGPIPE handler and perhaps install our own.
> */
> @@ -1210,6 +1216,7 @@
> if (!PQinSend())
> exit(128 + SIGPIPE); /* typical return value for SIG_DFL */
> }
> +#endif
> #endif
>
> /*

> /*-------------------------------------------------------------------------
> *
> * pthread-win32.c
> * partial pthread implementation for win32
> *
> * Copyright (c) 2004, PostgreSQL Global Development Group
> * IDENTIFICATION
> * $PostgreSQL: $
> *
> *-------------------------------------------------------------------------
> */
>
>
> #include "windows.h"
> #include "pthread.h"
>
> HANDLE pthread_self()
> {
> return GetCurrentThread();
> }
>
> void pthread_setspecific(pthread_key_t key, void *val)
> {
> }
>
> void *pthread_getspecific(pthread_key_t key)
> {
> return NULL;
> }
>
> void pthread_mutex_init(pthread_mutex_t *mp, void *attr)
> {
> *mp = CreateMutex(0, 0, 0);
> }
>
> void pthread_mutex_lock(pthread_mutex_t *mp)
> {
> WaitForSingleObject(*mp, INFINITE);
> }
>
> void pthread_mutex_unlock(pthread_mutex_t *mp)
> {
> ReleaseMutex(*mp);
> }

> #ifndef __PTHREAD_H
> #define __PTHREAD_H
>
> typedef ULONG pthread_key_t;
> typedef HANDLE pthread_mutex_t;
> typedef int pthread_once_t;
>
> HANDLE pthread_self();
>
> void pthread_setspecific(pthread_key_t, void*);
> void* pthread_getspecific(pthread_key_t);
>
> void pthread_mutex_init(pthread_mutex_t *, void *attr);
> void pthread_mutex_lock(pthread_mutex_t*); // blocking
> void pthread_mutex_unlock(pthread_mutex_t*);
>
> #endif

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

>I have looked over this patch. I noticed this:
>
> -static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
> -
> + static pthread_mutex_t init_mutex;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&init_mutex, NULL);
> + }
>
>While this might work using your pthread compatibility implementation
>using CreateMutex(), it will not work on a native pthread implementation
>because you can only call pthread_mutex_init() once. Your code allows
>two threads to both call it.
>
>
I don't really think so. That mutex_initialized is a globally static
variable, not a thread local one. Thread interruption between testing
mutex_initialized and setting it is very unlikely and still wouldn't do
much harm if it actually does happen.

>Also, do you not have the problem with SIGPIPE from send(), so you don't
>need set/get_thread_specific()?
>
>

There's no SIGPIPE under win32, thus no problem.

Regards,
Andreas

Andreas Pflug wrote:

> I don't really think so. That mutex_initialized is a globally static
> variable, not a thread local one. Thread interruption between testing
> mutex_initialized and setting it is very unlikely and still wouldn't
> do much harm if it actually does happen.
>
Very unlikely is not a good argument. And you haven't considered
multiprocessor systems. They aren't that rare: all newer Pentium 4
systems have two logical cores.
The harm would be a failed connection attempt - I don't think that this
is acceptable.

Hmm. Is libpq a .DLL? Then you could initialize the mutex in DllMain().
Otherwise create a C++ class with one instance and a constructor. Then
initialize the mutex from the constructor.

--
Manfred

Manfred Spraul wrote:
> Andreas Pflug wrote:
>
> > I don't really think so. That mutex_initialized is a globally static
> > variable, not a thread local one. Thread interruption between testing
> > mutex_initialized and setting it is very unlikely and still wouldn't
> > do much harm if it actually does happen.
> >
> Very unlikely is not a good argument. And you haven't considered
> multiprocessor systems. They aren't that rare: all newer Pentium 4
> systems have two logical cores.
> The harm would be a failed connection attempt - I don't think that this
> is acceptable.

Agreed. My pthread book says pthread_mutex_init() should be called only
once, and we have to guarantee that. If the Windows implentation allows
it to be called multiple times, just create a function to be called only
by Win32 that does that and leave the Unix safe.

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

>
>
>Agreed. My pthread book says pthread_mutex_init() should be called only
>once, and we have to guarantee that. If the Windows implentation allows
>it to be called multiple times, just create a function to be called only
>by Win32 that does that and leave the Unix safe.
>
>
>
Ok, so here's the win32 workaround with the unix stuff left untouched.
There's no memory interlocking api in win32 that wouldn't need some
initializing api call itself, so we'd have to go for assembly level
test-and-set code or introduce a mandatory global libpq initializing
api. Considering the probably quite low usage of kerberos/ssl together
with threads under win32, and the very low probability of two
threads/processors (!) trying to initiate a connection at the same time,
it doesn't seem to be worth the compiler hassle with assembly inline.

Regards,
Andreas

[ Thread moved to hackers and win32.]

Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >
> >
> >Agreed. My pthread book says pthread_mutex_init() should be called only
> >once, and we have to guarantee that. If the Windows implentation allows
> >it to be called multiple times, just create a function to be called only
> >by Win32 that does that and leave the Unix safe.
> >
> >
> >
> Ok, so here's the win32 workaround with the unix stuff left untouched.
> There's no memory interlocking api in win32 that wouldn't need some
> initializing api call itself, so we'd have to go for assembly level
> test-and-set code or introduce a mandatory global libpq initializing
> api. Considering the probably quite low usage of kerberos/ssl together
> with threads under win32, and the very low probability of two
> threads/processors (!) trying to initiate a connection at the same time,
> it doesn't seem to be worth the compiler hassle with assembly inline.

What is the recommended way to create mutex objects (CreateMutex) from
Win32 libraries? There must be a clean way like there is in pthreads.

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

In the patch Win32, pthread_mutex_init() == CreateMutex():

+#ifndef WIN32
static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
+#else
+ static pthread_mutex_t singlethread_lock;
+ static int mutex_initialized = 0;
+ if (!mutex_initialized)
+ {
+ mutex_initialized = 1;
+ pthread_mutex_init(&singlethread_lock, NULL);
+ }
+#endif

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

>
>What is the recommended way to create mutex objects (CreateMutex) from
>Win32 libraries? There must be a clean way like there is in pthreads.
>

It's having a central one-time called routine executing CreateMutex.
This can be DllMain, *if* used as DLL, but that's certainly no solution
for static linkage. This would require some PQinitThreadStuff() routine,
which may be called only once (and ultimately can't check for that
itself, if you don't trust a static var).

Regards,
Andreas

Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >
> >What is the recommended way to create mutex objects (CreateMutex) from
> >Win32 libraries? There must be a clean way like there is in pthreads.
> >
>
> It's having a central one-time called routine executing CreateMutex.
> This can be DllMain, *if* used as DLL, but that's certainly no solution
> for static linkage. This would require some PQinitThreadStuff() routine,
> which may be called only once (and ultimately can't check for that
> itself, if you don't trust a static var).

Ewe. Well, I am not excited about adding a thread capability for Win32
that isn't 100% guaranteed to work.

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

Andreas Pflug wrote:

> Bruce Momjian wrote:
>
>>
>>
>> Agreed. My pthread book says pthread_mutex_init() should be called only
>> once, and we have to guarantee that. If the Windows implentation allows
>> it to be called multiple times, just create a function to be called only
>> by Win32 that does that and leave the Unix safe.
>>
>>
>>
> Ok, so here's the win32 workaround with the unix stuff left untouched.
> There's no memory interlocking api in win32 that wouldn't need some
> initializing api call itself, so we'd have to go for assembly level
> test-and-set code.

Wrong. There are portable test-and-set functions in the Win32 SDK: for
example InterlockedCompareExchange. They operate on ints and do not need
initialization.

> or introduce a mandatory global libpq initializing api.

There is a third option: Add one C++ file and use the constructor to
initialize the mutex. VisualC is always a C++ compiler, thus C++ support
shouldn't be an issue. I've attached an example app.

> Considering the probably quite low usage of kerberos/ssl together with
> threads under win32, and the very low probability of two
> threads/processors (!) trying to initiate a connection at the same
> time, it doesn't seem to be worth the compiler hassle with assembly
> inline.
>
This argument is insane: Given enough installations, even a very low
probability will cause failures.

But this is a minor point, I think the patch should go in and I'll write
with a proposal to close the race, either based on
InterlockedCompareExchange or on a C++ file.

--
Manfred

Can you put that in the new pthread file that needs to be added for Win32?

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

Manfred Spraul wrote:
> Andreas Pflug wrote:
>
> > Bruce Momjian wrote:
> >
> >>
> >>
> >> Agreed. My pthread book says pthread_mutex_init() should be called only
> >> once, and we have to guarantee that. If the Windows implentation allows
> >> it to be called multiple times, just create a function to be called only
> >> by Win32 that does that and leave the Unix safe.
> >>
> >>
> >>
> > Ok, so here's the win32 workaround with the unix stuff left untouched.
> > There's no memory interlocking api in win32 that wouldn't need some
> > initializing api call itself, so we'd have to go for assembly level
> > test-and-set code.
>
> Wrong. There are portable test-and-set functions in the Win32 SDK: for
> example InterlockedCompareExchange. They operate on ints and do not need
> initialization.
>
> > or introduce a mandatory global libpq initializing api.
>
> There is a third option: Add one C++ file and use the constructor to
> initialize the mutex. VisualC is always a C++ compiler, thus C++ support
> shouldn't be an issue. I've attached an example app.
>
> > Considering the probably quite low usage of kerberos/ssl together with
> > threads under win32, and the very low probability of two
> > threads/processors (!) trying to initiate a connection at the same
> > time, it doesn't seem to be worth the compiler hassle with assembly
> > inline.
> >
> This argument is insane: Given enough installations, even a very low
> probability will cause failures.
>
> But this is a minor point, I think the patch should go in and I'll write
> with a proposal to close the race, either based on
> InterlockedCompareExchange or on a C++ file.
>
> --
> Manfred

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

Manfred Spraul wrote:

>
> Wrong. There are portable test-and-set functions in the Win32 SDK: for
> example InterlockedCompareExchange. They operate on ints and do not
> need initialization.

'k, missed that one. Lets use it.

>
> There is a third option: Add one C++ file and use the constructor to
> initialize the mutex. VisualC is always a C++ compiler, thus C++
> support shouldn't be an issue.

Our code is potentially not only VC.

Regards,
Andreas

Andreas Pflug wrote:
> Manfred Spraul wrote:
>
> >
> > Wrong. There are portable test-and-set functions in the Win32 SDK: for
> > example InterlockedCompareExchange. They operate on ints and do not
> > need initialization.
>
> 'k, missed that one. Lets use it.
>
> >
> > There is a third option: Add one C++ file and use the constructor to
> > initialize the mutex. VisualC is always a C++ compiler, thus C++
> > support shouldn't be an issue.
>
> Our code is potentially not only VC.

True, MinGW and Cygwin. Can we do the C solution?

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

This patch has to be redone to make it thread-safe.

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

Andreas Pflug wrote:
> The appended patch implements ENABLE_THREAD_SAFETY for win32 compiled
> with Visual C. Two additional files will supply the needed pthread stuff.
>
> There's no SIGPIPE for native win32, so there are some #IFNDEF WIN32 to
> skip installing a signal handler for this. I'm not sure if this is going
> to show conflicts for CYGWIN or other unix-like builds on win32
> platforms, i.e. if WIN32 is defined in such cases or not; please check.
>
> Note: the previously posted win32.mak patch is superseded by this.
>
> Regards,
> Andreas

> Index: win32.mak
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/win32.mak,v
> retrieving revision 1.23
> diff -u -r1.23 win32.mak
> --- win32.mak 3 Jun 2004 00:11:13 -0000 1.23
> +++ win32.mak 4 Jun 2004 13:26:39 -0000
> @@ -74,19 +74,25 @@
> -(at)erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
> -(at)erase "$(INTDIR)\wchar.obj"
> -(at)erase "$(INTDIR)\encnames.obj"
> + -(at)erase "$(INTDIR)\pthread-win32.obj"
>
>
>
> -config: ..\..\include\pg_config.h
> +config: ..\..\include\pg_config.h pthread.h pg_config_paths.h
>
> ..\..\include\pg_config.h: ..\..\include\pg_config.h.win32
> copy ..\..\include\pg_config.h.win32 ..\..\include\pg_config.h
>
> +pthread.h: pthread.h.win32
> + copy pthread.h.win32 pthread.h
> +
> +pg_config_paths.h: win32.mak
> + echo #define SYSCONFDIR "" >pg_config_paths.h
>
> "$(OUTDIR)" :
> if not exist "$(OUTDIR)/$(NULL)" mkdir "$(OUTDIR)"
>
> -CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /D "FRONTEND" $(DEBUGDEF) /D\
> +CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /I. /D "FRONTEND" $(DEBUGDEF) /D\
> "WIN32" /D "_WINDOWS" /Fp"$(INTDIR)\libpq.pch" /YX\
> /Fo"$(INTDIR)\\" /Fd"$(INTDIR)\\" /FD /c /D "HAVE_VSNPRINTF" /D "HAVE_STRDUP"
>
> @@ -95,6 +101,10 @@
> SSL_LIBS=ssleay32.lib libeay32.lib gdi32.lib
> !ENDIF
>
> +!IFDEF ENABLE_THREAD_SAFETY
> +CPP_PROJ=$(CPP_PROJ) /D ENABLE_THREAD_SAFETY
> +!ENDIF
> +
> CPP_SBRS=.
>
> LIB32=link.exe -lib
> @@ -121,7 +131,8 @@
> "$(INTDIR)\fe-secure.obj" \
> "$(INTDIR)\pqexpbuffer.obj" \
> "$(INTDIR)\wchar.obj" \
> - "$(INTDIR)\encnames.obj"
> + "$(INTDIR)\encnames.obj" \
> + "$(INTDIR)\pthread-win32.obj"
>
>
> RSC_PROJ=/l 0x409 /fo"$(INTDIR)\libpq.res"
> Index: fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.272
> diff -u -r1.272 fe-connect.c
> --- fe-connect.c 3 Jun 2004 00:07:38 -0000 1.272
> +++ fe-connect.c 4 Jun 2004 13:26:43 -0000
> @@ -882,11 +882,13 @@
> const char *node = NULL;
> int ret;
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
>
> /* Check only on first connection request */
> pthread_once(&check_sigpipe_once, check_sigpipe_handler);
> #endif
> +#endif
>
> if (!conn)
> return 0;
> @@ -3187,7 +3189,13 @@
> default_threadlock(int acquire)
> {
> #ifdef ENABLE_THREAD_SAFETY
> - static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
> + static pthread_mutex_t singlethread_lock;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&singlethread_lock, NULL);
> + }
> if (acquire)
> pthread_mutex_lock(&singlethread_lock);
> else
> Index: fe-secure.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
> retrieving revision 1.41
> diff -u -r1.41 fe-secure.c
> --- fe-secure.c 3 Jun 2004 00:13:19 -0000 1.41
> +++ fe-secure.c 4 Jun 2004 13:26:45 -0000
> @@ -864,8 +864,13 @@
> init_ssl_system(PGconn *conn)
> {
> #ifdef ENABLE_THREAD_SAFETY
> -static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
> -
> + static pthread_mutex_t init_mutex;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&init_mutex, NULL);
> + }
> pthread_mutex_lock(&init_mutex);
>
> if (pq_initssllib && pq_lockarray == NULL) {
> @@ -1171,6 +1176,7 @@
>
>
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> /*
> * Check SIGPIPE handler and perhaps install our own.
> */
> @@ -1210,6 +1216,7 @@
> if (!PQinSend())
> exit(128 + SIGPIPE); /* typical return value for SIG_DFL */
> }
> +#endif
> #endif
>
> /*

> /*-------------------------------------------------------------------------
> *
> * pthread-win32.c
> * partial pthread implementation for win32
> *
> * Copyright (c) 2004, PostgreSQL Global Development Group
> * IDENTIFICATION
> * $PostgreSQL: $
> *
> *-------------------------------------------------------------------------
> */
>
>
> #include "windows.h"
> #include "pthread.h"
>
> HANDLE pthread_self()
> {
> return GetCurrentThread();
> }
>
> void pthread_setspecific(pthread_key_t key, void *val)
> {
> }
>
> void *pthread_getspecific(pthread_key_t key)
> {
> return NULL;
> }
>
> void pthread_mutex_init(pthread_mutex_t *mp, void *attr)
> {
> *mp = CreateMutex(0, 0, 0);
> }
>
> void pthread_mutex_lock(pthread_mutex_t *mp)
> {
> WaitForSingleObject(*mp, INFINITE);
> }
>
> void pthread_mutex_unlock(pthread_mutex_t *mp)
> {
> ReleaseMutex(*mp);
> }

> #ifndef __PTHREAD_H
> #define __PTHREAD_H
>
> typedef ULONG pthread_key_t;
> typedef HANDLE pthread_mutex_t;
> typedef int pthread_once_t;
>
> HANDLE pthread_self();
>
> void pthread_setspecific(pthread_key_t, void*);
> void* pthread_getspecific(pthread_key_t);
>
> void pthread_mutex_init(pthread_mutex_t *, void *attr);
> void pthread_mutex_lock(pthread_mutex_t*); // blocking
> void pthread_mutex_unlock(pthread_mutex_t*);
>
> #endif

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

Sorry, I accidentally bounced this to the lists.

The patch needs redesign and has been removed from the patch queue.

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

Andreas Pflug wrote:
> The appended patch implements ENABLE_THREAD_SAFETY for win32 compiled
> with Visual C. Two additional files will supply the needed pthread stuff.
>
> There's no SIGPIPE for native win32, so there are some #IFNDEF WIN32 to
> skip installing a signal handler for this. I'm not sure if this is going
> to show conflicts for CYGWIN or other unix-like builds on win32
> platforms, i.e. if WIN32 is defined in such cases or not; please check.
>
> Note: the previously posted win32.mak patch is superseded by this.
>
> Regards,
> Andreas

> Index: win32.mak
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/win32.mak,v
> retrieving revision 1.23
> diff -u -r1.23 win32.mak
> --- win32.mak 3 Jun 2004 00:11:13 -0000 1.23
> +++ win32.mak 4 Jun 2004 13:26:39 -0000
> @@ -74,19 +74,25 @@
> -(at)erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
> -(at)erase "$(INTDIR)\wchar.obj"
> -(at)erase "$(INTDIR)\encnames.obj"
> + -(at)erase "$(INTDIR)\pthread-win32.obj"
>
>
>
> -config: ..\..\include\pg_config.h
> +config: ..\..\include\pg_config.h pthread.h pg_config_paths.h
>
> ..\..\include\pg_config.h: ..\..\include\pg_config.h.win32
> copy ..\..\include\pg_config.h.win32 ..\..\include\pg_config.h
>
> +pthread.h: pthread.h.win32
> + copy pthread.h.win32 pthread.h
> +
> +pg_config_paths.h: win32.mak
> + echo #define SYSCONFDIR "" >pg_config_paths.h
>
> "$(OUTDIR)" :
> if not exist "$(OUTDIR)/$(NULL)" mkdir "$(OUTDIR)"
>
> -CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /D "FRONTEND" $(DEBUGDEF) /D\
> +CPP_PROJ=/nologo /W3 /GX $(OPT) /I "..\..\include" /I. /D "FRONTEND" $(DEBUGDEF) /D\
> "WIN32" /D "_WINDOWS" /Fp"$(INTDIR)\libpq.pch" /YX\
> /Fo"$(INTDIR)\\" /Fd"$(INTDIR)\\" /FD /c /D "HAVE_VSNPRINTF" /D "HAVE_STRDUP"
>
> @@ -95,6 +101,10 @@
> SSL_LIBS=ssleay32.lib libeay32.lib gdi32.lib
> !ENDIF
>
> +!IFDEF ENABLE_THREAD_SAFETY
> +CPP_PROJ=$(CPP_PROJ) /D ENABLE_THREAD_SAFETY
> +!ENDIF
> +
> CPP_SBRS=.
>
> LIB32=link.exe -lib
> @@ -121,7 +131,8 @@
> "$(INTDIR)\fe-secure.obj" \
> "$(INTDIR)\pqexpbuffer.obj" \
> "$(INTDIR)\wchar.obj" \
> - "$(INTDIR)\encnames.obj"
> + "$(INTDIR)\encnames.obj" \
> + "$(INTDIR)\pthread-win32.obj"
>
>
> RSC_PROJ=/l 0x409 /fo"$(INTDIR)\libpq.res"
> Index: fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.272
> diff -u -r1.272 fe-connect.c
> --- fe-connect.c 3 Jun 2004 00:07:38 -0000 1.272
> +++ fe-connect.c 4 Jun 2004 13:26:43 -0000
> @@ -882,11 +882,13 @@
> const char *node = NULL;
> int ret;
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
>
> /* Check only on first connection request */
> pthread_once(&check_sigpipe_once, check_sigpipe_handler);
> #endif
> +#endif
>
> if (!conn)
> return 0;
> @@ -3187,7 +3189,13 @@
> default_threadlock(int acquire)
> {
> #ifdef ENABLE_THREAD_SAFETY
> - static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
> + static pthread_mutex_t singlethread_lock;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&singlethread_lock, NULL);
> + }
> if (acquire)
> pthread_mutex_lock(&singlethread_lock);
> else
> Index: fe-secure.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
> retrieving revision 1.41
> diff -u -r1.41 fe-secure.c
> --- fe-secure.c 3 Jun 2004 00:13:19 -0000 1.41
> +++ fe-secure.c 4 Jun 2004 13:26:45 -0000
> @@ -864,8 +864,13 @@
> init_ssl_system(PGconn *conn)
> {
> #ifdef ENABLE_THREAD_SAFETY
> -static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
> -
> + static pthread_mutex_t init_mutex;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&init_mutex, NULL);
> + }
> pthread_mutex_lock(&init_mutex);
>
> if (pq_initssllib && pq_lockarray == NULL) {
> @@ -1171,6 +1176,7 @@
>
>
> #ifdef ENABLE_THREAD_SAFETY
> +#ifndef WIN32
> /*
> * Check SIGPIPE handler and perhaps install our own.
> */
> @@ -1210,6 +1216,7 @@
> if (!PQinSend())
> exit(128 + SIGPIPE); /* typical return value for SIG_DFL */
> }
> +#endif
> #endif
>
> /*

> /*-------------------------------------------------------------------------
> *
> * pthread-win32.c
> * partial pthread implementation for win32
> *
> * Copyright (c) 2004, PostgreSQL Global Development Group
> * IDENTIFICATION
> * $PostgreSQL: $
> *
> *-------------------------------------------------------------------------
> */
>
>
> #include "windows.h"
> #include "pthread.h"
>
> HANDLE pthread_self()
> {
> return GetCurrentThread();
> }
>
> void pthread_setspecific(pthread_key_t key, void *val)
> {
> }
>
> void *pthread_getspecific(pthread_key_t key)
> {
> return NULL;
> }
>
> void pthread_mutex_init(pthread_mutex_t *mp, void *attr)
> {
> *mp = CreateMutex(0, 0, 0);
> }
>
> void pthread_mutex_lock(pthread_mutex_t *mp)
> {
> WaitForSingleObject(*mp, INFINITE);
> }
>
> void pthread_mutex_unlock(pthread_mutex_t *mp)
> {
> ReleaseMutex(*mp);
> }

> #ifndef __PTHREAD_H
> #define __PTHREAD_H
>
> typedef ULONG pthread_key_t;
> typedef HANDLE pthread_mutex_t;
> typedef int pthread_once_t;
>
> HANDLE pthread_self();
>
> void pthread_setspecific(pthread_key_t, void*);
> void* pthread_getspecific(pthread_key_t);
>
> void pthread_mutex_init(pthread_mutex_t *, void *attr);
> void pthread_mutex_lock(pthread_mutex_t*); // blocking
> void pthread_mutex_unlock(pthread_mutex_t*);
>
> #endif

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

>
> [ Thread moved to hackers and win32.]
>
> Andreas Pflug wrote:
>> Bruce Momjian wrote:
>>
>> >
>> >
>> >Agreed. My pthread book says pthread_mutex_init() should be called
>> only
>> >once, and we have to guarantee that. If the Windows implentation
>> allows
>> >it to be called multiple times, just create a function to be called
>> only
>> >by Win32 that does that and leave the Unix safe.
>> >
>> >
>> >
>> Ok, so here's the win32 workaround with the unix stuff left untouched.
>> There's no memory interlocking api in win32 that wouldn't need some
>> initializing api call itself, so we'd have to go for assembly level
>> test-and-set code or introduce a mandatory global libpq initializing
>> api. Considering the probably quite low usage of kerberos/ssl together
>> with threads under win32, and the very low probability of two
>> threads/processors (!) trying to initiate a connection at the same time,
>> it doesn't seem to be worth the compiler hassle with assembly inline.
>
> What is the recommended way to create mutex objects (CreateMutex) from
> Win32 libraries? There must be a clean way like there is in pthreads.

A mutex is inherently a global object. CreateMutex(NULL, FALSE, NULL) will
return a handle to an unowned mutex.

>
> ---------------------------------------------------------------------------
>
> In the patch Win32, pthread_mutex_init() == CreateMutex():
>
> +#ifndef WIN32
> static pthread_mutex_t singlethread_lock =
> PTHREAD_MUTEX_INITIALIZER;
> +#else
> + static pthread_mutex_t singlethread_lock;
> + static int mutex_initialized = 0;
> + if (!mutex_initialized)
> + {
> + mutex_initialized = 1;
> + pthread_mutex_init(&singlethread_lock, NULL);
> + }
> +#endif
>
> --
> 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
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

pgsql(at)mohawksoft(dot)com wrote:
> >
> > [ Thread moved to hackers and win32.]
> >
> > Andreas Pflug wrote:
> >> Bruce Momjian wrote:
> >>
> >> >
> >> >
> >> >Agreed. My pthread book says pthread_mutex_init() should be called
> >> only
> >> >once, and we have to guarantee that. If the Windows implentation
> >> allows
> >> >it to be called multiple times, just create a function to be called
> >> only
> >> >by Win32 that does that and leave the Unix safe.
> >> >
> >> >
> >> >
> >> Ok, so here's the win32 workaround with the unix stuff left untouched.
> >> There's no memory interlocking api in win32 that wouldn't need some
> >> initializing api call itself, so we'd have to go for assembly level
> >> test-and-set code or introduce a mandatory global libpq initializing
> >> api. Considering the probably quite low usage of kerberos/ssl together
> >> with threads under win32, and the very low probability of two
> >> threads/processors (!) trying to initiate a connection at the same time,
> >> it doesn't seem to be worth the compiler hassle with assembly inline.
> >
> > What is the recommended way to create mutex objects (CreateMutex) from
> > Win32 libraries? There must be a clean way like there is in pthreads.
>
> A mutex is inherently a global object. CreateMutex(NULL, FALSE, NULL) will
> return a handle to an unowned mutex.

Yes, but consider that two threads could both call it, with one perhaps
using the first value, and the second overwriting the first. Obviously
not something we want.

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

pgsql(at)mohawksoft(dot)com wrote:

>>
>>What is the recommended way to create mutex objects (CreateMutex) from
>>Win32 libraries? There must be a clean way like there is in pthreads.
>>
>>
>
>A mutex is inherently a global object. CreateMutex(NULL, FALSE, NULL) will
>return a handle to an unowned mutex.
>
>
>
That's not the problem. Under pthread, it's possible to initialize a
mutex from compile time:

static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;

This means that the mutex is immediately valid, no races with the
initialization. I couldn't find an equivalent Win32 feature.

--
Manfred

On 13 Jun 2004 at 19:57, Bruce Momjian wrote:

>
> Yes, but consider that two threads could both call it, with one perhaps
> using the first value, and the second overwriting the first. Obviously
> not something we want.
>

Every Windows DLL has (or can have) a DllMain function. This is called
everytime a Dll is attached to (loaded) by a process or thread. You can
put any one-time initialisation in this function and guarantee that it is
only called by the first process/thread that loads the dll.

BOOL WINAPI DllMain(
HINSTANCE hinstDLL,
DWORD fdwReason,
LPVOID lpvReserved
);

Parameters:

hinstDLL
[in] Handle to the DLL module. The value is the base address of the
DLL. The HINSTANCE of a DLL is the same as the HMODULE of the
DLL, so hinstDLL can be used in calls to functions that require a
module handle.

fdwReason
[in] Indicates why the DLL entry-point function is being called. This
parameter can be one of the following values. Value Meaning

DLL_PROCESS_ATTACH The DLL is being loaded into the virtual
address space of the current process as a result of the process starting
up or as a result of a call to LoadLibrary. DLLs can use this opportunity
to initialize any instance data or to use the TlsAlloc function to allocate
a thread local storage (TLS) index.

DLL_THREAD_ATTACH The current process is creating a new thread.
When this occurs, the system calls the entry-point function of all DLLs
currently attached to the process. The call is made in the context of the
new thread. DLLs can use this opportunity to initialize a TLS slot for the
thread. A thread calling the DLL entry-point function with
DLL_PROCESS_ATTACH does not call the DLL entry-point function
with DLL_THREAD_ATTACH.
Note that a DLL's entry-point function is called with this value only by
threads created after the DLL is loaded by the process. When a DLL is
loaded using LoadLibrary, existing threads do not call the entry-point
function of the newly loaded DLL.

DLL_THREAD_DETACH A thread is exiting cleanly. If the DLL has
stored a pointer to allocated memory in a TLS slot, it should use this
opportunity to free the memory. The system calls the entry-point
function of all currently loaded DLLs with this value. The call is made in
the context of the exiting thread.

DLL_PROCESS_DETACH The DLL is being unloaded from the virtual
address space of the calling process as a result of unsuccessfully
loading the DLL, termination of the process, or a call to FreeLibrary.
The DLL can use this opportunity to call the TlsFree function to free any
TLS indices allocated by using TlsAlloc and to free any thread local
data.
Note that the thread that receives the DLL_PROCESS_DETACH
notification is not necessarily the same thread that received the
DLL_PROCESS_ATTACH notification.

lpvReserved
[in] If fdwReason is DLL_PROCESS_ATTACH, lpvReserved is NULL
for dynamic loads and non-NULL for static loads.
If fdwReason is DLL_PROCESS_DETACH, lpvReserved is NULL if
DllMain has been called by using FreeLibrary and non-NULL if DllMain
has been called during process termination.

Regards,
Gary

Two minor tweaks:

<sys/time.h> in libpq-be.h isn't present on win32 vc6. Apparently, it
isn't used in Linux either; libpq will compile without it. All usages of
gettimeofday throughout the backend don't seem connection related, so
libpq-be.h seems an odd place to include it.

ERROR in elog.h has conflicts with a win32 defined macro; #undef WIN32
solves the conflict (or not including elog.h for libpq purposes would do
too)

Regards,
Andreas

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> <sys/time.h> in libpq-be.h isn't present on win32 vc6. Apparently, it
> isn't used in Linux either; libpq will compile without it.

It's there to declare struct timeval, and I'm fairly certain that diking
it out of the header would break things on some platforms. Where does
Windows define struct timeval?

> +#ifndef WIN32
> #include <sys/time.h>
> +#endif

Could we please use HAVE_SYS_TIME_H, rather than creating an unnecessary
platform specificity?

> #ifndef ELOG_H
> #define ELOG_H

> +#ifdef ERROR
> +#undef ERROR /* possible conflict in win32 */
> +#endif
> +
> /* Error level codes */

This seems likely to break anything expecting the Win32 definition
of ERROR. Can we look at not including elog.h in frontend builds,
instead? I can't think of any reason for frontend code to need it.

regards, tom lane

Tom Lane wrote:
> Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> > <sys/time.h> in libpq-be.h isn't present on win32 vc6. Apparently, it
> > isn't used in Linux either; libpq will compile without it.
>
> It's there to declare struct timeval, and I'm fairly certain that diking
> it out of the header would break things on some platforms. Where does
> Windows define struct timeval?
>
> > +#ifndef WIN32
> > #include <sys/time.h>
> > +#endif
>
> Could we please use HAVE_SYS_TIME_H, rather than creating an unnecessary
> platform specificity?

The problem is that they are building from win32.mak, and don't run
configure at all. Mingw does have it so we can't just skip including
it. I think we should test for the MS compiler in this case. Please
test for _MSC_VER instead of WIN32 and let us know how it works.

> > #ifndef ELOG_H
> > #define ELOG_H
>
> > +#ifdef ERROR
> > +#undef ERROR /* possible conflict in win32 */
> > +#endif
> > +
> > /* Error level codes */
>
> This seems likely to break anything expecting the Win32 definition
> of ERROR. Can we look at not including elog.h in frontend builds,
> instead? I can't think of any reason for frontend code to need it.

Agreed. We define FRONTEND when compiling libpq. Please test for that
and send another patch.

Thanks.

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

Tom wrote:

>>It's there to declare struct timeval, and I'm fairly certain that diking
>>it out of the header would break things on some platforms. Where does
>>Windows define struct timeval?
>>
>>
struct timeval is defined in winsock.h under vc6.
I'm checking for _MSC_VER now.

>Agreed. We define FRONTEND when compiling libpq. Please test for that
>and send another patch.
>
>
elog.h is included in postgres.h, which is included in many
src/port/*.c. Many of them are pretty straight, not requiring any
backend specific stuff, so the attached patch will change postgres.h to
c.h for most of them.

Regards,
Andreas

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> elog.h is included in postgres.h, which is included in many
> src/port/*.c. Many of them are pretty straight, not requiring any
> backend specific stuff, so the attached patch will change postgres.h to
> c.h for most of them.

Applied. I did the sys/time.h stuff the standard Autoconf way, however.

regards, tom lane