database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-03 22:24:49 +03:00
Code

Convert libpgport's pqsignal() to a void function.

Browse Source 
The protections added by commit 3b00fdba9f introduced race
conditions to this function that can lead to bogus return values.
Since nobody seems to inspect the return value, this is of little
consequence, but it would have been nice to convert it to a void
function to avoid any possibility of a bogus return value.  I
originally thought that doing so would have required also modifying
legacy-pqsignal.c's version of the function (which would've
required an SONAME bump), but commit 9a45a89c38 gave
legacy-pqsignal.c its own dedicated extern for pqsignal(), thereby
decoupling it enough that libpgport's pqsignal() can be modified.

This commit also adds an assertion for the return value of
sigaction()/signal().  Since a failure most likely indicates a
coding error, and nobody has ever bothered to check pqsignal()'s
return value, it's probably not worth the effort to do anything
fancier.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
This commit is contained in:
Nathan Bossart 2025-01-16 16:41:05 -06:00
parent 5cda4fdb0b
commit d4a43b2837
2 changed files with 7 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/include/port.h
View File

@ -520,7 +520,7 @@ extern int pg_mkdir_p(char *path, int omode);
#define pqsignal pqsignal_be
#endif
typedef void (*pqsigfunc) (SIGNAL_ARGS);
extern pqsigfunc pqsignal(int signo, pqsigfunc func);
extern void pqsignal(int signo, pqsigfunc func);
/* port/quotes.c */
extern char *escape_single_quotes_ascii(const char *src);

34
src/port/pqsignal.c
View File

@ -112,31 +112,15 @@ wrapper_handler(SIGNAL_ARGS)
/*
* Set up a signal handler, with SA_RESTART, for signal "signo"
*
* Returns the previous handler.
*
* NB: If called within a signal handler, race conditions may lead to bogus
* return values. You should either avoid calling this within signal handlers
* or ignore the return value.
*
* XXX: Since no in-tree callers use the return value, and there is little
* reason to do so, it would be nice if we could convert this to a void
* function instead of providing potentially-bogus return values.
* Unfortunately, that requires modifying the pqsignal() in legacy-pqsignal.c,
* which in turn requires an SONAME bump, which is probably not worth it.
*
* Note: the actual name of this function is either pqsignal_fe when
* compiled with -DFRONTEND, or pqsignal_be when compiled without that.
* This is to avoid a name collision with libpq's legacy-pqsignal.c.
*/
pqsigfunc
void
pqsignal(int signo, pqsigfunc func)
{
pqsigfunc orig_func = pqsignal_handlers[signo]; /* assumed atomic */
#if !(defined(WIN32) && defined(FRONTEND))
struct sigaction act,
oact;
#else
pqsigfunc ret;
struct sigaction act;
#endif
Assert(signo < PG_NSIG);
@ -155,17 +139,11 @@ pqsignal(int signo, pqsigfunc func)
if (signo == SIGCHLD)
act.sa_flags |= SA_NOCLDSTOP;
#endif
if (sigaction(signo, &act, &oact) < 0)
return SIG_ERR;
else if (oact.sa_handler == wrapper_handler)
return orig_func;
else
return oact.sa_handler;
if (sigaction(signo, &act, NULL) < 0)
Assert(false); /* probably indicates coding error */
#else
/* Forward to Windows native signal system. */
if ((ret = signal(signo, func)) == wrapper_handler)
return orig_func;
else
return ret;
if (signal(signo, func) == SIG_ERR)
Assert(false); /* probably indicates coding error */
#endif
}