Skip to content

Commit 56c0d7a

Browse files
committed
Remove GLV unlocking at all functions which process data modifiable in a second thread
This removes possible VM crashs when data to be sent is modified/cleared in a second thread. It works by keeping the GVL lock for libpq functions that don't immediately process all the data and don't make a copy of it. These are the `PQsend*`, `PQexec*` and some related functions. Since pg-1.3 all the blocking functions or states are avoided by using the non-blocking API of libpq. Therefore holding the GVL somewhat longer shouldn't matter that much. Having some libpq function with and without unlocked GVL, results in `rb_thread_call_with_gvl()` sometimes needed and sometimes not to process callbacks. Therefore `ruby_thread_has_gvl_p()` is used to check if it's needed on ruby<4.0. In ruby-4.0+ `rb_thread_call_with_gvl()` doesn't care about whether GVL is already locked or not, so that it can be called in both cases. Fixes #721
1 parent 14d7216 commit 56c0d7a

4 files changed

Lines changed: 65 additions & 147 deletions

File tree

ext/gvl_wrappers.h

Lines changed: 33 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515
#ifndef __gvl_wrappers_h
1616
#define __gvl_wrappers_h
1717

18+
#include <ruby/version.h>
1819
#include <ruby/thread.h>
1920

2021
#ifdef RUBY_EXTCONF_H
2122
# include RUBY_EXTCONF_H
2223
#endif
2324

25+
#if RUBY_API_VERSION_MAJOR < 4
26+
extern int ruby_thread_has_gvl_p(void);
27+
#endif
28+
2429
#ifndef LIBPQ_HAS_CHUNK_MODE
2530
typedef struct pg_cancel_conn PGcancelConn;
2631
#endif
@@ -83,20 +88,35 @@ typedef struct pg_cancel_conn PGcancelConn;
8388
}
8489

8590
#ifdef ENABLE_GVL_UNLOCK
86-
#define DEFINE_GVLCB_STUB(name, when_non_void, rettype, lastparamtype, lastparamname) \
87-
rettype gvl_##name(FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST3) lastparamtype lastparamname){ \
88-
struct gvl_wrapper_##name##_params params = { \
89-
{FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST1) lastparamname}, when_non_void((rettype)0) \
90-
}; \
91-
rb_thread_call_with_gvl(gvl_##name##_skeleton, &params); \
92-
when_non_void( return params.retval; ) \
93-
}
91+
#if RUBY_API_VERSION_MAJOR >= 4 || defined(TRUFFLERUBY)
92+
#define DEFINE_GVLCB_STUB(name, when_non_void, rettype, lastparamtype, lastparamname) \
93+
rettype gvl_##name(FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST3) lastparamtype lastparamname){ \
94+
struct gvl_wrapper_##name##_params params = { \
95+
{FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST1) lastparamname}, when_non_void((rettype)0) \
96+
}; \
97+
rb_thread_call_with_gvl(gvl_##name##_skeleton, &params); \
98+
when_non_void( return params.retval; ) \
99+
}
100+
#else
101+
#define DEFINE_GVLCB_STUB(name, when_non_void, rettype, lastparamtype, lastparamname) \
102+
rettype gvl_##name(FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST3) lastparamtype lastparamname){ \
103+
struct gvl_wrapper_##name##_params params = { \
104+
{FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST1) lastparamname}, when_non_void((rettype)0) \
105+
}; \
106+
if (ruby_thread_has_gvl_p()) { \
107+
gvl_##name##_skeleton(&params); \
108+
} else { \
109+
rb_thread_call_with_gvl(gvl_##name##_skeleton, &params); \
110+
} \
111+
when_non_void( return params.retval; ) \
112+
}
113+
#endif
94114
#else
95-
#define DEFINE_GVLCB_STUB(name, when_non_void, rettype, lastparamtype, lastparamname) \
96-
rettype gvl_##name(FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST3) lastparamtype lastparamname){ \
97-
when_non_void( return ) \
98-
name( FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST1) lastparamname ); \
99-
}
115+
#define DEFINE_GVLCB_STUB(name, when_non_void, rettype, lastparamtype, lastparamname) \
116+
rettype gvl_##name(FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST3) lastparamtype lastparamname){ \
117+
when_non_void( return ) \
118+
name( FOR_EACH_PARAM_OF_##name(DEFINE_PARAM_LIST1) lastparamname ); \
119+
}
100120
#endif
101121

102122
#define GVL_TYPE_VOID(string)
@@ -121,50 +141,8 @@ typedef struct pg_cancel_conn PGcancelConn;
121141

122142
#define FOR_EACH_PARAM_OF_PQping(param)
123143

124-
#define FOR_EACH_PARAM_OF_PQexec(param) \
125-
param(PGconn *, conn)
126-
127-
#define FOR_EACH_PARAM_OF_PQexecParams(param) \
128-
param(PGconn *, conn) \
129-
param(const char *, command) \
130-
param(int, nParams) \
131-
param(const Oid *, paramTypes) \
132-
param(const char * const *, paramValues) \
133-
param(const int *, paramLengths) \
134-
param(const int *, paramFormats)
135-
136-
#define FOR_EACH_PARAM_OF_PQexecPrepared(param) \
137-
param(PGconn *, conn) \
138-
param(const char *, stmtName) \
139-
param(int, nParams) \
140-
param(const char * const *, paramValues) \
141-
param(const int *, paramLengths) \
142-
param(const int *, paramFormats)
143-
144-
#define FOR_EACH_PARAM_OF_PQprepare(param) \
145-
param(PGconn *, conn) \
146-
param(const char *, stmtName) \
147-
param(const char *, query) \
148-
param(int, nParams)
149-
150-
#define FOR_EACH_PARAM_OF_PQdescribePrepared(param) \
151-
param(PGconn *, conn)
152-
153-
#define FOR_EACH_PARAM_OF_PQdescribePortal(param) \
154-
param(PGconn *, conn)
155-
156-
#define FOR_EACH_PARAM_OF_PQclosePrepared(param) \
157-
param(PGconn *, conn)
158-
159-
#define FOR_EACH_PARAM_OF_PQclosePortal(param) \
160-
param(PGconn *, conn)
161-
162144
#define FOR_EACH_PARAM_OF_PQgetResult(param)
163145

164-
#define FOR_EACH_PARAM_OF_PQputCopyData(param) \
165-
param(PGconn *, conn) \
166-
param(const char *, buffer)
167-
168146
#define FOR_EACH_PARAM_OF_PQputCopyEnd(param) \
169147
param(PGconn *, conn)
170148

@@ -174,48 +152,8 @@ typedef struct pg_cancel_conn PGcancelConn;
174152

175153
#define FOR_EACH_PARAM_OF_PQnotifies(param)
176154

177-
#define FOR_EACH_PARAM_OF_PQsendQuery(param) \
178-
param(PGconn *, conn)
179-
180-
#define FOR_EACH_PARAM_OF_PQsendQueryParams(param) \
181-
param(PGconn *, conn) \
182-
param(const char *, command) \
183-
param(int, nParams) \
184-
param(const Oid *, paramTypes) \
185-
param(const char *const *, paramValues) \
186-
param(const int *, paramLengths) \
187-
param(const int *, paramFormats)
188-
189-
#define FOR_EACH_PARAM_OF_PQsendPrepare(param) \
190-
param(PGconn *, conn) \
191-
param(const char *, stmtName) \
192-
param(const char *, query) \
193-
param(int, nParams)
194-
195-
#define FOR_EACH_PARAM_OF_PQsendQueryPrepared(param) \
196-
param(PGconn *, conn) \
197-
param(const char *, stmtName) \
198-
param(int, nParams) \
199-
param(const char *const *, paramValues) \
200-
param(const int *, paramLengths) \
201-
param(const int *, paramFormats)
202-
203-
#define FOR_EACH_PARAM_OF_PQsendDescribePrepared(param) \
204-
param(PGconn *, conn)
205-
206-
#define FOR_EACH_PARAM_OF_PQsendDescribePortal(param) \
207-
param(PGconn *, conn)
208-
209-
#define FOR_EACH_PARAM_OF_PQsendClosePrepared(param) \
210-
param(PGconn *, conn)
211-
212-
#define FOR_EACH_PARAM_OF_PQsendClosePortal(param) \
213-
param(PGconn *, conn)
214-
215155
#define FOR_EACH_PARAM_OF_PQpipelineSync(param)
216156

217-
#define FOR_EACH_PARAM_OF_PQsendPipelineSync(param)
218-
219157
#define FOR_EACH_PARAM_OF_PQsetClientEncoding(param) \
220158
param(PGconn *, conn)
221159

@@ -225,11 +163,6 @@ typedef struct pg_cancel_conn PGcancelConn;
225163
#define FOR_EACH_PARAM_OF_PQcancelStart(param)
226164
#define FOR_EACH_PARAM_OF_PQcancelPoll(param)
227165

228-
#define FOR_EACH_PARAM_OF_PQencryptPasswordConn(param) \
229-
param(PGconn *, conn) \
230-
param(const char *, passwd) \
231-
param(const char *, user)
232-
233166
#define FOR_EACH_PARAM_OF_PQcancel(param) \
234167
param(PGcancel *, cancel) \
235168
param(char *, errbuf)
@@ -243,35 +176,16 @@ typedef struct pg_cancel_conn PGcancelConn;
243176
function(PQresetStart, GVL_TYPE_NONVOID, int, PGconn *, conn) \
244177
function(PQresetPoll, GVL_TYPE_NONVOID, PostgresPollingStatusType, PGconn *, conn) \
245178
function(PQping, GVL_TYPE_NONVOID, PGPing, const char *, conninfo) \
246-
function(PQexec, GVL_TYPE_NONVOID, PGresult *, const char *, command) \
247-
function(PQexecParams, GVL_TYPE_NONVOID, PGresult *, int, resultFormat) \
248-
function(PQexecPrepared, GVL_TYPE_NONVOID, PGresult *, int, resultFormat) \
249-
function(PQprepare, GVL_TYPE_NONVOID, PGresult *, const Oid *, paramTypes) \
250-
function(PQdescribePrepared, GVL_TYPE_NONVOID, PGresult *, const char *, stmtName) \
251-
function(PQdescribePortal, GVL_TYPE_NONVOID, PGresult *, const char *, portalName) \
252-
function(PQclosePrepared, GVL_TYPE_NONVOID, PGresult *, const char *, stmtName) \
253-
function(PQclosePortal, GVL_TYPE_NONVOID, PGresult *, const char *, portalName) \
254179
function(PQgetResult, GVL_TYPE_NONVOID, PGresult *, PGconn *, conn) \
255-
function(PQputCopyData, GVL_TYPE_NONVOID, int, int, nbytes) \
256180
function(PQputCopyEnd, GVL_TYPE_NONVOID, int, const char *, errormsg) \
257181
function(PQgetCopyData, GVL_TYPE_NONVOID, int, int, async) \
258182
function(PQnotifies, GVL_TYPE_NONVOID, PGnotify *, PGconn *, conn) \
259-
function(PQsendQuery, GVL_TYPE_NONVOID, int, const char *, query) \
260-
function(PQsendQueryParams, GVL_TYPE_NONVOID, int, int, resultFormat) \
261-
function(PQsendPrepare, GVL_TYPE_NONVOID, int, const Oid *, paramTypes) \
262-
function(PQsendQueryPrepared, GVL_TYPE_NONVOID, int, int, resultFormat) \
263-
function(PQsendDescribePrepared, GVL_TYPE_NONVOID, int, const char *, stmt) \
264-
function(PQsendDescribePortal, GVL_TYPE_NONVOID, int, const char *, portal) \
265-
function(PQsendClosePrepared, GVL_TYPE_NONVOID, int, const char *, stmt) \
266-
function(PQsendClosePortal, GVL_TYPE_NONVOID, int, const char *, portal) \
267183
function(PQpipelineSync, GVL_TYPE_NONVOID, int, PGconn *, conn) \
268-
function(PQsendPipelineSync, GVL_TYPE_NONVOID, int, PGconn *, conn) \
269184
function(PQsetClientEncoding, GVL_TYPE_NONVOID, int, const char *, encoding) \
270185
function(PQisBusy, GVL_TYPE_NONVOID, int, PGconn *, conn) \
271186
function(PQcancelBlocking, GVL_TYPE_NONVOID, int, PGcancelConn *, conn) \
272187
function(PQcancelStart, GVL_TYPE_NONVOID, int, PGcancelConn *, conn) \
273188
function(PQcancelPoll, GVL_TYPE_NONVOID, PostgresPollingStatusType, PGcancelConn *, conn) \
274-
function(PQencryptPasswordConn, GVL_TYPE_NONVOID, char *, const char *, algorithm) \
275189
function(PQcancel, GVL_TYPE_NONVOID, int, int, errbufsize);
276190

277191
FOR_EACH_BLOCKING_FUNCTION( DEFINE_GVL_STUB_DECL );

ext/pg.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ typedef long suseconds_t;
8282

8383
#define pg_gc_location(x) x = rb_gc_location(x)
8484

85+
extern int ruby_native_thread_p(void);
86+
8587
/* For compatibility with ruby < 3.0 */
8688
#ifndef RUBY_TYPED_FROZEN_SHAREABLE
8789
#define PG_RUBY_TYPED_FROZEN_SHAREABLE 0

0 commit comments

Comments
 (0)