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

Commitc6d9012

Browse files
committed
Fix incorrect order of lock file removal and failure to close() sockets.
Commitc9b0cbe accidentally broke theorder of operations during postmaster shutdown: it resulted in removingthe per-socket lockfiles after, not before, postmaster.pid. This createsa race-condition hazard for a new postmaster that's started immediatelyafter observing that postmaster.pid has disappeared; if it sees thesocket lockfile still present, it will quite properly refuse to start.This error appears to be the explanation for at least some of theintermittent buildfarm failures we've seen in the pg_upgrade test.Another problem, which has been there all along, is that the postmasterhas never bothered to close() its listen sockets, but has just allowed themto close at process death. This creates a different race condition for anincoming postmaster: it might be unable to bind to the desired listenaddress because the old postmaster is still incumbent. This might explainsome odd failures we've seen in the past, too. (Note: this is not relatedto the fact that individual backends don't close their client communicationsockets. That behavior is intentional and is not changed by this patch.)Fix by adding an on_proc_exit function that closes the postmaster's portsexplicitly, and (in 9.3 and up) reshuffling the responsibility for whereto unlink the Unix socket files. Lock file unlinking can stay where itis, but teach it to unlink the lock files in reverse order of creation.
1 parentbab9599 commitc6d9012

File tree

4 files changed

+75
-30
lines changed

4 files changed

+75
-30
lines changed

‎src/backend/libpq/pqcomm.c

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -247,28 +247,6 @@ pq_close(int code, Datum arg)
247247
*/
248248

249249

250-
/* StreamDoUnlink()
251-
* Shutdown routine for backend connection
252-
* If any Unix sockets are used for communication, explicitly close them.
253-
*/
254-
#ifdefHAVE_UNIX_SOCKETS
255-
staticvoid
256-
StreamDoUnlink(intcode,Datumarg)
257-
{
258-
ListCell*l;
259-
260-
/* Loop through all created sockets... */
261-
foreach(l,sock_paths)
262-
{
263-
char*sock_path= (char*)lfirst(l);
264-
265-
unlink(sock_path);
266-
}
267-
/* Since we're about to exit, no need to reclaim storage */
268-
sock_paths=NIL;
269-
}
270-
#endif/* HAVE_UNIX_SOCKETS */
271-
272250
/*
273251
* StreamServerPort -- open a "listening" port to accept connections.
274252
*
@@ -550,16 +528,11 @@ Lock_AF_UNIX(char *unixSocketDir, char *unixSocketPath)
550528
* Once we have the interlock, we can safely delete any pre-existing
551529
* socket file to avoid failure at bind() time.
552530
*/
553-
unlink(unixSocketPath);
531+
(void)unlink(unixSocketPath);
554532

555533
/*
556-
* Arrange to unlink the socket file(s) at proc_exit. If this is the
557-
* first one, set up the on_proc_exit function to do it; then add this
558-
* socket file to the list of files to unlink.
534+
* Remember socket file pathnames for later maintenance.
559535
*/
560-
if (sock_paths==NIL)
561-
on_proc_exit(StreamDoUnlink,0);
562-
563536
sock_paths=lappend(sock_paths,pstrdup(unixSocketPath));
564537

565538
returnSTATUS_OK;
@@ -788,6 +761,26 @@ TouchSocketFiles(void)
788761
}
789762
}
790763

764+
/*
765+
* RemoveSocketFiles -- unlink socket files at postmaster shutdown
766+
*/
767+
void
768+
RemoveSocketFiles(void)
769+
{
770+
ListCell*l;
771+
772+
/* Loop through all created sockets... */
773+
foreach(l,sock_paths)
774+
{
775+
char*sock_path= (char*)lfirst(l);
776+
777+
/* Ignore any error. */
778+
(void)unlink(sock_path);
779+
}
780+
/* Since we're about to exit, no need to reclaim storage */
781+
sock_paths=NIL;
782+
}
783+
791784

792785
/* --------------------------------
793786
* Low-level I/O routines begin here.

‎src/backend/postmaster/postmaster.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ static DNSServiceRef bonjour_sdref = NULL;
370370
/*
371371
* postmaster.c - function prototypes
372372
*/
373+
staticvoidCloseServerPorts(intstatus,Datumarg);
373374
staticvoidunlink_external_pid_file(intstatus,Datumarg);
374375
staticvoidgetInstallationPaths(constchar*argv0);
375376
staticvoidcheckDataDir(void);
@@ -892,6 +893,11 @@ PostmasterMain(int argc, char *argv[])
892893
* interlock (thanks to whoever decided to put socket files in /tmp :-().
893894
* For the same reason, it's best to grab the TCP socket(s) before the
894895
* Unix socket(s).
896+
*
897+
* Also note that this internally sets up the on_proc_exit function that
898+
* is responsible for removing both data directory and socket lockfiles;
899+
* so it must happen before opening sockets so that at exit, the socket
900+
* lockfiles go away after CloseServerPorts runs.
895901
*/
896902
CreateDataDirLockFile(true);
897903

@@ -916,10 +922,15 @@ PostmasterMain(int argc, char *argv[])
916922

917923
/*
918924
* Establish input sockets.
925+
*
926+
* First, mark them all closed, and set up an on_proc_exit function that's
927+
* charged with closing the sockets again at postmaster shutdown.
919928
*/
920929
for (i=0;i<MAXLISTEN;i++)
921930
ListenSocket[i]=PGINVALID_SOCKET;
922931

932+
on_proc_exit(CloseServerPorts,0);
933+
923934
if (ListenAddresses)
924935
{
925936
char*rawstring;
@@ -1263,6 +1274,42 @@ PostmasterMain(int argc, char *argv[])
12631274
}
12641275

12651276

1277+
/*
1278+
* on_proc_exit callback to close server's listen sockets
1279+
*/
1280+
staticvoid
1281+
CloseServerPorts(intstatus,Datumarg)
1282+
{
1283+
inti;
1284+
1285+
/*
1286+
* First, explicitly close all the socket FDs. We used to just let this
1287+
* happen implicitly at postmaster exit, but it's better to close them
1288+
* before we remove the postmaster.pid lockfile; otherwise there's a race
1289+
* condition if a new postmaster wants to re-use the TCP port number.
1290+
*/
1291+
for (i=0;i<MAXLISTEN;i++)
1292+
{
1293+
if (ListenSocket[i]!=PGINVALID_SOCKET)
1294+
{
1295+
StreamClose(ListenSocket[i]);
1296+
ListenSocket[i]=PGINVALID_SOCKET;
1297+
}
1298+
}
1299+
1300+
/*
1301+
* Next, remove any filesystem entries for Unix sockets. To avoid race
1302+
* conditions against incoming postmasters, this must happen after closing
1303+
* the sockets and before removing lock files.
1304+
*/
1305+
RemoveSocketFiles();
1306+
1307+
/*
1308+
* We don't do anything about socket lock files here; those will be
1309+
* removed in a later on_proc_exit callback.
1310+
*/
1311+
}
1312+
12661313
/*
12671314
* on_proc_exit callback to delete external_pid_file
12681315
*/

‎src/backend/utils/init/miscinit.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,11 @@ CreateLockFile(const char *filename, bool amPostmaster,
884884
if (lock_files==NIL)
885885
on_proc_exit(UnlinkLockFiles,0);
886886

887-
lock_files=lappend(lock_files,pstrdup(filename));
887+
/*
888+
* Use lcons so that the lock files are unlinked in reverse order of
889+
* creation; this is critical!
890+
*/
891+
lock_files=lcons(pstrdup(filename),lock_files);
888892
}
889893

890894
/*

‎src/include/libpq/libpq.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ extern int StreamServerPort(int family, char *hostName,
5050
externintStreamConnection(pgsocketserver_fd,Port*port);
5151
externvoidStreamClose(pgsocketsock);
5252
externvoidTouchSocketFiles(void);
53+
externvoidRemoveSocketFiles(void);
5354
externvoidpq_init(void);
5455
externvoidpq_comm_reset(void);
5556
externintpq_getbytes(char*s,size_tlen);

0 commit comments

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