8000 Clean up Windows-specific mutex code in libpq and ecpglib. · postgres/postgres@0028b55 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0028b55

Browse files
tglsfdctglsfdc
committed
Clean up Windows-specific mutex code in libpq and ecpglib.
Fix pthread-win32.h and pthread-win32.c to provide a more complete emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER and make sure that pthread_mutex_lock() can operate on a mutex object that's been initialized that way. Then we don't need the duplicative platform-specific logic in default_threadlock() and pgtls_init(), which we'd otherwise need yet a third copy of for an upcoming bug fix. Also, since default_threadlock() supposes that pthread_mutex_lock() cannot fail, try to ensure that that's actually true, by getting rid of the malloc call that was formerly involved in initializing an emulated mutex. We can define an extra state for the spinlock field instead. Also, replace the similar code in ecpglib/misc.c with this version. While ecpglib's version at least had a POSIX-compliant API, it also had the potential of failing during mutex init (but here, because of CreateMutex failure rather than malloc failure). Since all of misc.c's callers ignore failures, it seems like a wise idea to avoid failures here too. A further improvement in this area could be to unify libpq's and ecpglib's implementations into a src/port/pthread-win32.c file. But that doesn't seem like a bug fix, so I'll desist for now. In preparation for the aforementioned bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us
1 parent 5c7038d commit 0028b55

File tree

6 files changed

+63
-69
lines changed

6 files changed

+63
-69
lines changed

‎src/interfaces/ecpg/ecpglib/misc.c

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -407,17 +407,38 @@ ECPGis_noind_null(enum ECPGttype type, const void *ptr)
407407

408408
#ifdef WIN32
409409

410-
void
411-
win32_pthread_mutex(volatile pthread_mutex_t *mutex)
410+
int
411+
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
412+
{
413+
mp->initstate = 0;
414+
return 0;
415+
}
416+
417+
int
418+
pthread_mutex_lock(pthread_mutex_t *mp)
412419
{
413-
if (mutex->handle == NULL)
420+
/* Initialize the csection if not already done */
421+
if (mp->initstate != 1)
414422
{
415-
while (InterlockedExchange((LONG *) &mutex->initlock, 1) == 1)
416-
Sleep(0);
417-
if (mutex->handle == NULL)
418-
mutex->handle = CreateMutex(NULL, FALSE, NULL);
419-
InterlockedExchange((LONG *) &mutex->initlock, 0);
423+
LONG istate;
424+
425+
while ((istate = InterlockedExchange(&mp->initstate, 2)) == 2)
426+
Sleep(0); /* wait, another thread is doing this */
427+
if (istate != 1)
428+
InitializeCriticalSection(&mp->csection);
429+
InterlockedExchange(&mp->initstate, 1);
420430
}
431+
EnterCriticalSection(&mp->csection);
432+
return 0;
433+
}
434+
435+
int
436+
pthread_mutex_unlock(pthread_mutex_t *mp)
437+
{
438+
if (mp->initstate != 1)
439+
return EINVAL;
440+
LeaveCriticalSection(&mp->csection);
441+
return 0;
421442
}
422443

423444
static pthread_mutex_t win32_pthread_once_lock = PTHREAD_MUTEX_INITIALIZER;

‎src/interfaces/ecpg/include/ecpg-pthread-win32.h

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,22 @@
1212

1313
typedef struct pthread_mutex_t
1414
{
15-
HANDLE handle;
16-
LONG initlock;
15+
/* initstate = 0: not initialized; 1: init done; 2: init in progress */
16+
LONG initstate;
17+
CRITICAL_SECTION csection;
1718
} pthread_mutex_t;
1819

1920
typedef DWORD pthread_key_t;
2021
typedef bool pthread_once_t;
2122

22-
#define PTHREAD_MUTEX_INITIALIZER { NULL, 0 }
23+
#define PTHREAD_MUTEX_INITIALIZER { 0 }
2324
#define PTHREAD_ONCE_INIT false
2425

25-
void win32_pthread_mutex(volatile pthread_mutex_t *mutex);
26-
void win32_pthread_once(volatile pthread_once_t *once, void (*fn) (void));
26+
int pthread_mutex_init(pthread_mutex_t *, void *attr);
27+
int pthread_mutex_lock(pthread_mutex_t *);
28+
int pthread_mutex_unlock(pthread_mutex_t *);
2729

28-
#define pthread_mutex_lock(mutex) \
29-
do { \
30-
if ((mutex)->handle == NULL) \
31-
win32_pthread_mutex((mutex)); \
32-
WaitForSingleObject((mutex)->handle, INFINITE); \
33-
} while(0)
34-
35-
#define pthread_mutex_unlock(mutex) \
36-
ReleaseMutex((mutex)->handle)
30+
void win32_pthread_once(volatile pthread_once_t *once, void (*fn) (void));
3731

3832
#define pthread_getspecific(key) \
3933
TlsGetValue((key))

‎src/interfaces/libpq/fe-connect.c

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7426,24 +7426,8 @@ pqParseIntParam(const char *value, int *result, PGconn *conn,
74267426
static void
74277427
default_threadlock(int acquire)
74287428
{
7429-
#ifndef WIN32
74307429
static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
7431-
#else
7432-
static pthread_mutex_t singlethread_lock = NULL;
7433-
static long mutex_initlock = 0;
74347430

7435-
if (singlethread_lock == NULL)
7436-
{
7437-
while (InterlockedExchange(&mutex_initlock, 1) == 1)
7438-
/* loop, another thread own the lock */ ;
7439-
if (singlethread_lock == NULL)
7440-
{
7441-
if (pthread_mutex_init(&singlethread_lock, NULL))
7442-
Assert(false);
7443-
}
7444-
InterlockedExchange(&mutex_initlock, 0);
7445-
}
7446-
#endif
74477431
if (acquire)
74487432
{
74497433
if (pthread_mutex_lock(&singlethread_lock))

‎src/interfaces/libpq/fe-secure-openssl.c

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,7 @@ static bool ssl_lib_initialized = false;
9191

9292
static long crypto_open_connections = 0;
9393

94-
#ifndef WIN32
9594
static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
96-
#else
97-
static pthread_mutex_t ssl_config_mutex = NULL;
98-
static long win32_ssl_create_mutex = 0;
99-
#endif
10095

10196
static PQsslKeyPassHook_OpenSSL_type PQsslKeyPassHook = NULL;
10297
static int ssl_protocol_version_to_openssl(const char *protocol);
@@ -773,20 +768,6 @@ pq_lockingcallback(int mode, int n, const char *file, int line)
773768
int
774769
pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto)
775770
{
776-
#ifdef WIN32
777-
/* Also see similar code in fe-connect.c, default_threadlock() */
778-
if (ssl_config_mutex == NULL)
779-
{
780-
while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
781-
/* loop, another thread own the lock */ ;
782-
if (ssl_config_mutex == NULL)
783-
{
784-
if (pthread_mutex_init(&ssl_config_mutex, NULL))
785-
return -1;
786-
}
787-
InterlockedExchange(&win32_ssl_create_mutex, 0);
788-
}
789-
#endif
790771
if (pthread_mutex_lock(&ssl_config_mutex))
791772
return -1;
792773

@@ -874,7 +855,6 @@ static void
874855
destroy_ssl_system(void)
875856
{
876857
#if defined(HAVE_CRYPTO_LOCK)
877-
/* Mutex is created in pgtls_init() */
878858
if (pthread_mutex_lock(&ssl_config_mutex))
879859
return;
880860

‎src/interfaces/libpq/pthread-win32.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,33 @@ pthread_getspecific(pthread_key_t key)
3434
int
3535
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
3636
{
37-
*mp = (CRITICAL_SECTION *) malloc(sizeof(CRITICAL_SECTION));
38-
if (!*mp)
39-
return 1;
40-
InitializeCriticalSection(*mp);
37+
mp->initstate = 0;
4138
return 0;
4239
}
4340

4441
int
4542
pthread_mutex_lock(pthread_mutex_t *mp)
4643
{
47-
if (!*mp)
48-
return 1;
49-
EnterCriticalSection(*mp);
44+
/* Initialize the csection if not already done */
45+
if (mp->initstate != 1)
46+
{
47+
LONG istate;
48+
49+
while ((istate = InterlockedExchange(&mp->initstate, 2)) == 2)
50+
Sleep(0); /* wait, another thread is doing this */
51+
if (istate != 1)
52+
InitializeCriticalSection(&mp->csection);
53+
InterlockedExchange(&mp->initstate, 1);
54+
}
55+
EnterCriticalSection(&mp->csection);
5056
return 0;
5157
}
5258

5359
int
5460
pthread_mutex_unlock(pthread_mutex_t *mp)
5561
{
56-
if (!*mp)
57-
return 1;
58-
LeaveCriticalSection(*mp);
62+
if (mp->initstate != 1)
63+
return EINVAL;
64+
LeaveCriticalSection(&mp->csection);
5965
return 0;
6066
}

‎src/port/pthread-win32.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,16 @@
55
#define __PTHREAD_H
66

77
typedef ULONG pthread_key_t;
8-
typedef CRITICAL_SECTION *pthread_mutex_t;
8+
9+
typedef struct pthread_mutex_t
10+
{
11+
/* initstate = 0: not initialized; 1: init done; 2: init in progress */
12+
LONG initstate;
13+
CRITICAL_SECTION csection;
14+
} pthread_mutex_t;
15+
16+
#define PTHREAD_MUTEX_INITIALIZER { 0 }
17+
918
typedef int pthread_once_t;
1019

1120
DWORD pthread_self(void);

0 commit comments

Comments
 (0)
0