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

Commite892e72

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 parentc0df15a commite892e72

File tree

1 file changed

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

1 file changed

+30
-11
lines changed

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ static struct sqlca_t sqlca =
9191
staticpthread_mutex_tdebug_mutex=PTHREAD_MUTEX_INITIALIZER;
9292
staticpthread_mutex_tdebug_init_mutex=PTHREAD_MUTEX_INITIALIZER;
9393
#endif
94-
staticintsimple_debug=0;
94+
staticvolatileintsimple_debug=0;
9595
staticFILE*debugstream=NULL;
9696

9797
void
@@ -241,7 +241,11 @@ void
241241
ECPGdebug(intn,FILE*dbgs)
242242
{
243243
#ifdefENABLE_THREAD_SAFETY
244+
/* Interlock against concurrent executions of ECPGdebug() */
244245
pthread_mutex_lock(&debug_init_mutex);
246+
247+
/* Prevent ecpg_log() from printing while we change settings */
248+
pthread_mutex_lock(&debug_mutex);
245249
#endif
246250

247251
if (n>100)
@@ -254,6 +258,12 @@ ECPGdebug(int n, FILE *dbgs)
254258

255259
debugstream=dbgs;
256260

261+
/* We must release debug_mutex before invoking ecpg_log() ... */
262+
#ifdefENABLE_THREAD_SAFETY
263+
pthread_mutex_unlock(&debug_mutex);
264+
#endif
265+
266+
/* ... but keep holding debug_init_mutex to avoid racy printout */
257267
ecpg_log("ECPGdebug: set to %d\n",simple_debug);
258268

259269
#ifdefENABLE_THREAD_SAFETY
@@ -270,6 +280,11 @@ ecpg_log(const char *format,...)
270280
intbufsize;
271281
char*fmt;
272282

283+
/*
284+
* For performance reasons, inspect simple_debug without taking the mutex.
285+
* This could be problematic if fetching an int isn't atomic, but we
286+
* assume that it is in many other places too.
287+
*/
273288
if (!simple_debug)
274289
return;
275290

@@ -294,18 +309,22 @@ ecpg_log(const char *format,...)
294309
pthread_mutex_lock(&debug_mutex);
295310
#endif
296311

297-
va_start(ap,format);
298-
vfprintf(debugstream,fmt,ap);
299-
va_end(ap);
300-
301-
/* dump out internal sqlca variables */
302-
if (ecpg_internal_regression_mode&&sqlca!=NULL)
312+
/* Now that we hold the mutex, recheck simple_debug */
313+
if (simple_debug)
303314
{
304-
fprintf(debugstream,"[NO_PID]: sqlca: code: %ld, state: %s\n",
305-
sqlca->sqlcode,sqlca->sqlstate);
306-
}
315+
va_start(ap,format);
316+
vfprintf(debugstream,fmt,ap);
317+
va_end(ap);
307318

308-
fflush(debugstream);
319+
/* dump out internal sqlca variables */
320+
if (ecpg_internal_regression_mode&&sqlca!=NULL)
321+
{
322+
fprintf(debugstream,"[NO_PID]: sqlca: code: %ld, state: %s\n",
323+
sqlca->sqlcode,sqlca->sqlstate);
324+
}
325+
326+
fflush(debugstream);
327+
}
309328

310329
#ifdefENABLE_THREAD_SAFETY
311330
pthread_mutex_unlock(&debug_mutex);

0 commit comments

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