Thread: ecpg threading vs win32
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)
/* $PostgreSQL$ */
/*
* pthread mapping macros for win32 native thread implementation
*/
#ifndef _ECPG_PTHREAD_WIN32_H
#define _ECPG_PTHREAD_WIN32_H
#define pthread_mutex_lock(x) do { \
if (*x == INVALID_HANDLE_VALUE) \
*x = CreateMutex(NULL, FALSE, NULL); \
WaitForSingleObject(*x, INFINITE); \
} while (0);
#define pthread_mutex_unlock(x) ReleaseMutex(*x)
#define pthread_getspecific(x) TlsGetValue(x)
#define pthread_setspecific(x,y) TlsSetValue(x,y)
#define pthread_key_create(x,y) *x = TlsAlloc();
#endif
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/img/article/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/pgsql/sgml/libpq-exec.html#LIBPQ-EXEC-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
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/img/article/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/pgsql/sgml/libpq-exec.html#LIBPQ-EXEC-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
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/pgsql/sgml/libpq-exec.html#LIBPQ-EXEC-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!
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/pgsql/sgml/libpq-exec.html#LIBPQ-EXEC-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. +
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!