Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0162a9b

Browse files
committed
Remove race conditions between ECPGdebug() and ecpg_log().
Coverity complains that ECPGdebug is accessing debugstream withoutholding debug_mutex, which is a fair complaint: we should takedebug_mutex while changing the settings ecpg_log looks at.In some branches it also complains about unlocked use of simple_debug.I think it's intentional and safe to have a quick unlocked check ofsimple_debug at the start of ecpg_log, since that early exit willalways be taken in non-debug cases. But we should rechecksimple_debug after acquiring the mutex. In the worst case, callingECPGdebug concurrently with ecpg_log in another thread could resultin a null-pointer dereference due to debugstream transiently beingNULL while simple_debug isn't 0.This is largely hypothetical, since it's unlikely anybody usesECPGdebug() at all in the field, and our own regression testsdon't seem to be hitting the theoretical race conditions either.Still, if we're going to the trouble of having mutexes here, we oughtto be using them in a way that's actually safe not just almost safe.Hence, back-patch to all supported branches.
1 parentda32f5c commit0162a9b

File tree

1 file changed

+28
-11
lines changed
  • src/interfaces/ecpg/ecpglib

1 file changed

+28
-11
lines changed

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

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ static pthread_once_t sqlca_key_once = PTHREAD_ONCE_INIT;
6060

6161
staticpthread_mutex_tdebug_mutex=PTHREAD_MUTEX_INITIALIZER;
6262
staticpthread_mutex_tdebug_init_mutex=PTHREAD_MUTEX_INITIALIZER;
63-
staticintsimple_debug=0;
63+
staticvolatileintsimple_debug=0;
6464
staticFILE*debugstream=NULL;
6565

6666
void
@@ -203,8 +203,12 @@ ECPGtrans(int lineno, const char *connection_name, const char *transaction)
203203
void
204204
ECPGdebug(intn,FILE*dbgs)
205205
{
206+
/* Interlock against concurrent executions of ECPGdebug() */
206207
pthread_mutex_lock(&debug_init_mutex);
207208

209+
/* Prevent ecpg_log() from printing while we change settings */
210+
pthread_mutex_lock(&debug_mutex);
211+
208212
if (n>100)
209213
{
210214
ecpg_internal_regression_mode= true;
@@ -215,6 +219,10 @@ ECPGdebug(int n, FILE *dbgs)
215219

216220
debugstream=dbgs;
217221

222+
/* We must release debug_mutex before invoking ecpg_log() ... */
223+
pthread_mutex_unlock(&debug_mutex);
224+
225+
/* ... but keep holding debug_init_mutex to avoid racy printout */
218226
ecpg_log("ECPGdebug: set to %d\n",simple_debug);
219227

220228
pthread_mutex_unlock(&debug_init_mutex);
@@ -229,6 +237,11 @@ ecpg_log(const char *format,...)
229237
intbufsize;
230238
char*fmt;
231239

240+
/*
241+
* For performance reasons, inspect simple_debug without taking the mutex.
242+
* This could be problematic if fetching an int isn't atomic, but we
243+
* assume that it is in many other places too.
244+
*/
232245
if (!simple_debug)
233246
return;
234247

@@ -251,18 +264,22 @@ ecpg_log(const char *format,...)
251264

252265
pthread_mutex_lock(&debug_mutex);
253266

254-
va_start(ap,format);
255-
vfprintf(debugstream,fmt,ap);
256-
va_end(ap);
257-
258-
/* dump out internal sqlca variables */
259-
if (ecpg_internal_regression_mode&&sqlca!=NULL)
267+
/* Now that we hold the mutex, recheck simple_debug */
268+
if (simple_debug)
260269
{
261-
fprintf(debugstream,"[NO_PID]: sqlca: code: %ld, state: %s\n",
262-
sqlca->sqlcode,sqlca->sqlstate);
263-
}
270+
va_start(ap,format);
271+
vfprintf(debugstream,fmt,ap);
272+
va_end(ap);
273+
274+
/* dump out internal sqlca variables */
275+
if (ecpg_internal_regression_mode&&sqlca!=NULL)
276+
{
277+
fprintf(debugstream,"[NO_PID]: sqlca: code: %ld, state: %s\n",
278+
sqlca->sqlcode,sqlca->sqlstate);
279+
}
264280

265-
fflush(debugstream);
281+
fflush(debugstream);
282+
}
266283

267284
pthread_mutex_unlock(&debug_mutex);
268285

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp