This patch replaces the pthreads code in ecpg with native win32 threads,
in order to make it threadsafe. The idea is not to have to download the
non-standard pthreads library on windows.

Does it seem like it should be doing the right thing? Does somebody have
a good test-case where ecpg breaks when not built thread-safe? (which
would then also break when built thread-safe with a broken implementation)

//Magnus

Index: connect.c
================================================== =================
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/ecpglib/connect.c,v
retrieving revision 1.39
diff -u -r1.39 connect.c
--- connect.c 12 Jan 2007 10:00:12 -0000 1.39
+++ connect.c 14 Mar 2007 12:47:48 -0000
@@ -4,7 +4,11 @@
#include "postgres_fe.h"

#ifdef ENABLE_THREAD_SAFETY
+#ifndef WIN32
#include <pthread.h>
+#else
+#include "ecpg-pthread-win32.h"
+#endif
#endif
#include "ecpgtype.h"
#include "ecpglib.h"
@@ -13,9 +17,14 @@
#include "sqlca.h"

#ifdef ENABLE_THREAD_SAFETY
+#ifndef WIN32
static pthread_mutex_t connections_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_key_t actual_connection_key;
static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;
+#else
+static HANDLE connections_mutex = INVALID_HANDLE_VALUE;
+static DWORD actual_connection_key;
+#endif /* WIN32 */
#endif
static struct connection *actual_connection = NULL;
static struct connection *all_connections = NULL;
@@ -30,7 +39,13 @@
void
ecpg_pthreads_init(void)
{
+#ifndef WIN32
pthread_once(&actual_connection_key_once, ecpg_actual_connection_init);
+#else
+ static long has_run = 0;
+ if (InterlockedCompareExchange(&has_run, 1, 0) == 0)
+ ecpg_actual_connection_init();
+#endif
}
#endif

Index: misc.c
================================================== =================
RCS file: /projects/cvsroot/pgsql/src/interfaces/ecpg/ecpglib/misc.c,v
retrieving revision 1.34
diff -u -r1.34 misc.c
--- misc.c 12 Jan 2007 10:00:13 -0000 1.34
+++ misc.c 14 Mar 2007 12:48:03 -0000
@@ -6,7 +6,11 @@
#include <limits.h>
#include <unistd.h>
#ifdef ENABLE_THREAD_SAFETY
+#ifndef WIN32
#include <pthread.h>
+#else
+#include "ecpg-pthread-win32.h"
+#endif
#endif
#include "ecpgtype.h"
#include "ecpglib.h"
@@ -58,9 +62,13 @@
};

#ifdef ENABLE_THREAD_SAFETY
+#ifndef WIN32
static pthread_key_t sqlca_key;
static pthread_once_t sqlca_key_once = PTHREAD_ONCE_INIT;
#else
+static DWORD sqlca_key;
+#endif
+#else
static struct sqlca_t sqlca =
{
{
@@ -90,8 +98,13 @@
#endif

#ifdef ENABLE_THREAD_SAFETY
+#ifndef WIN32
static pthread_mutex_t debug_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t debug_init_mutex = PTHREAD_MUTEX_INITIALIZER;
+#else
+static HANDLE debug_mutex = INVALID_HANDLE_VALUE;
+static HANDLE debug_init_mutex = INVALID_HANDLE_VALUE;
+#endif /* WIN32 */
#endif
static int simple_debug = 0;
static FILE *debugstream = NULL;
@@ -138,8 +151,13 @@
{
#ifdef ENABLE_THREAD_SAFETY
struct sqlca_t *sqlca;
-
+#ifdef WIN32
+ static long has_run = 0;
+ if (InterlockedCompareExchange(&has_run, 1, 0) == 0)
+ ecpg_sqlca_key_init();
+#else
pthread_once(&sqlca_key_once, ecpg_sqlca_key_init);
+#endif

sqlca = pthread_getspecific(sqlca_key);
if (sqlca == NULL)



---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Magnus Hagander <magnus@hagander.net> wrote:

> This patch replaces the pthreads code in ecpg with native win32 threads,
> in order to make it threadsafe. The idea is not to have to download the
> non-standard pthreads library on windows.
>
> Does it seem like it should be doing the right thing? Does somebody have
> a good test-case where ecpg breaks when not built thread-safe? (which
> would then also break when built thread-safe with a broken implementation)

I have two questions about thread-safe ecpg.

Q1. Don't you use CRITICAL_SECTION instead of Mutex (CreateMutex)?
I've heard there is a performance benefit in CRITICAL_SECTION.
If the mutex is shared only in one process, CS might be a better solution.
http://japan.internet.com/developer/.../873/17801.gif
http://world.std.com/~jmhart/csmutx.htm

Q2. Do we need to use PQescapeStringConn() instead of PQescapeString()?
PQescapeString() is used to escape literals, and the documentation says
PQescapeStringConn() should be used in multi-threaded client programs.
http://momjian.us/main/writings/pgsq...-ESCAPE-STRING
| PQescapeString can be used safely in single-threaded client programs
| that work with only one PostgreSQL connection at a time



Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



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

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

On Mon, Mar 19, 2007 at 09:33:54AM +0900, ITAGAKI Takahiro wrote:
>
> Magnus Hagander <magnus@hagander.net> wrote:
>
> > This patch replaces the pthreads code in ecpg with native win32 threads,
> > in order to make it threadsafe. The idea is not to have to download the
> > non-standard pthreads library on windows.
> >
> > Does it seem like it should be doing the right thing? Does somebody have
> > a good test-case where ecpg breaks when not built thread-safe? (which
> > would then also break when built thread-safe with a broken implementation)
>
> I have two questions about thread-safe ecpg.
>
> Q1. Don't you use CRITICAL_SECTION instead of Mutex (CreateMutex)?
> I've heard there is a performance benefit in CRITICAL_SECTION.
> If the mutex is shared only in one process, CS might be a better solution.
> http://japan.internet.com/developer/.../873/17801.gif
> http://world.std.com/~jmhart/csmutx.htm

Yes, CS can be slightly faster. Though under this use-pattern, I think
it will not make a measurable difference at all.
The reason I went with Mutex is that I wanted it to be as similar as
possible to the pthreads code.

> Q2. Do we need to use PQescapeStringConn() instead of PQescapeString()?
> PQescapeString() is used to escape literals, and the documentation says
> PQescapeStringConn() should be used in multi-threaded client programs.
> http://momjian.us/main/writings/pgsq...-ESCAPE-STRING
> | PQescapeString can be used safely in single-threaded client programs
> | that work with only one PostgreSQL connection at a time

Seems so, but that's unrelated to this patch ;-) I'll leave the final
comment on that up to Michael.

//Magnus


---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

On Mon, Mar 19, 2007 at 09:48:19AM +0100, Magnus Hagander wrote:
> > Q2. Do we need to use PQescapeStringConn() instead of PQescapeString()?
> > PQescapeString() is used to escape literals, and the documentation says
> > PQescapeStringConn() should be used in multi-threaded client programs.
> > http://momjian.us/main/writings/pgsq...-ESCAPE-STRING
> > | PQescapeString can be used safely in single-threaded client programs
> > | that work with only one PostgreSQL connection at a time
>
> Seems so, but that's unrelated to this patch ;-) I'll leave the final
> comment on that up to Michael.

Looking at the source code it seems to me that the connection argument
is only used once in PQescapeStringInternal which both functions call.
Here's the snippet:

if (conn)
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("incomplete multibyte character\n"));

So this essantially is only to get an error message into the connection
structure. However, having an empty connection makes PQescapeStringConn
return an error and an empty string which I consider a problem. I have
to check that in detail but we may run into a usage of
PQescapeString without an open connection which then would fail.

Michael
--
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Michael, is there any progress on this?

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

Michael Meskes wrote:
> On Mon, Mar 19, 2007 at 09:48:19AM +0100, Magnus Hagander wrote:
> > > Q2. Do we need to use PQescapeStringConn() instead of PQescapeString()?
> > > PQescapeString() is used to escape literals, and the documentation says
> > > PQescapeStringConn() should be used in multi-threaded client programs.
> > > http://momjian.us/main/writings/pgsq...-ESCAPE-STRING
> > > | PQescapeString can be used safely in single-threaded client programs
> > > | that work with only one PostgreSQL connection at a time
> >
> > Seems so, but that's unrelated to this patch ;-) I'll leave the final
> > comment on that up to Michael.
>
> Looking at the source code it seems to me that the connection argument
> is only used once in PQescapeStringInternal which both functions call.
> Here's the snippet:
>
> if (conn)
> printfPQExpBuffer(&conn->errorMessage,
> libpq_gettext("incomplete multibyte character\n"));
>
> So this essantially is only to get an error message into the connection
> structure. However, having an empty connection makes PQescapeStringConn
> return an error and an empty string which I consider a problem. I have
> to check that in detail but we may run into a usage of
> PQescapeString without an open connection which then would fail.
>
> Michael
> --
> Michael Meskes
> Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
> ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
> Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
> http://www.postgresql.org/about/donate

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

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

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

On Mon, Apr 02, 2007 at 09:06:17PM -0400, Bruce Momjian wrote:
>
> Michael, is there any progress on this?
> ---------------------------------------------------------------------------
> > So this essantially is only to get an error message into the connection
> > structure. However, having an empty connection makes PQescapeStringConn
> > return an error and an empty string which I consider a problem. I have
> > to check that in detail but we may run into a usage of
> > PQescapeString without an open connection which then would fail.

Not really. IMO it should remain as it is.

Michael
--
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate