diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index df844ff..a9d7bf9 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -352,9 +352,9 @@ PGSharedMemoryAttach(IpcMemoryId shmId, return SHMSTATE_ENOENT; /* - * EACCES implies that the segment belongs to some other userid, which - * means it is not a Postgres shmem segment (or at least, not one that - * is relevant to our data directory). + * EACCES implies we have no read permission, which means it is not a + * Postgres shmem segment (or at least, not one that is relevant to + * our data directory). */ if (errno == EACCES) return SHMSTATE_FOREIGN; @@ -388,13 +388,19 @@ PGSharedMemoryAttach(IpcMemoryId shmId, return SHMSTATE_ANALYSIS_FAILURE; /* can't stat; be conservative */ /* - * If we can't attach, be conservative. This may fail if postmaster.pid - * furnished the shmId and another user created a world-readable segment - * of the same shmId. + * Attachment fails if we have no write permission. Since that will never + * happen with Postgres IPCProtection, such a failure shows the segment is + * not a Postgres segment. If attachment fails for some other reason, be + * conservative. */ hdr = (PGShmemHeader *) shmat(shmId, UsedShmemSegAddr, PG_SHMAT_FLAGS); if (hdr == (PGShmemHeader *) -1) - return SHMSTATE_ANALYSIS_FAILURE; + { + if (errno == EACCES) + return SHMSTATE_FOREIGN; + else + return SHMSTATE_ANALYSIS_FAILURE; + } *addr = hdr; if (hdr->magic != PGShmemMagic || diff --git a/src/test/recovery/t/017_shm.pl b/src/test/recovery/t/017_shm.pl index 8e85d6b..0969d4a 100644 --- a/src/test/recovery/t/017_shm.pl +++ b/src/test/recovery/t/017_shm.pl @@ -7,8 +7,9 @@ use IPC::Run 'run'; use PostgresNode; use Test::More; use TestLib; +use Time::HiRes qw(usleep); -plan tests => 6; +plan tests => 5; my $tempdir = TestLib::tempdir; my $port; @@ -23,31 +24,40 @@ sub log_ipcs return; } -# With Unix sockets, choose a port number such that the port number's first -# IpcMemoryKey candidate is available. If multiple copies of this test run -# concurrently, they will pick different ports. In the absence of collisions -# from other shmget() activity, gnat starts with key 0x7d001 (512001), and -# flea starts with key 0x7d002 (512002). With TCP, the first get_new_node -# picks a port number. +# These tests need a $port such that nothing creates or removes a segment in +# $port's IpcMemoryKey range while this test script runs. While there's no +# way to ensure that in general, we do ensure that if PostgreSQL tests are the +# only actors. With TCP, the first get_new_node picks a port number. With +# Unix sockets, use a postmaster, $port_holder, to represent a key space +# reservation. $port_holder holds a reservation on the key space of port +# 1+$port_holder->port if it created the first IpcMemoryKey of its own port's +# key space. If multiple copies of this test script run concurrently, they +# will pick different ports. $port_holder postmasters use odd-numbered ports, +# and tests use even-numbered ports. In the absence of collisions from other +# shmget() activity, gnat starts with key 0x7d001 (512001), and flea starts +# with key 0x7d002 (512002). my $port_holder; if (!$PostgresNode::use_tcp) { - for ($port = 512; $port < 612; ++$port) + my $lock_port; + for ($lock_port = 511; $lock_port < 711; $lock_port += 2) { $port_holder = PostgresNode->get_new_node( - "port${port}_holder", - port => $port, + "port${lock_port}_holder", + port => $lock_port, own_host => 1); $port_holder->init; + $port_holder->append_conf('postgresql.conf', 'max_connections = 5'); $port_holder->start; # Match the AddToDataDirLockFile() call in sysv_shmem.c. Assume all # systems not using sysv_shmem.c do use TCP. - my $shmem_key_line_prefix = sprintf("%9lu ", 1 + $port * 1000); + my $shmem_key_line_prefix = sprintf("%9lu ", 1 + $lock_port * 1000); last if slurp_file($port_holder->data_dir . '/postmaster.pid') =~ /^$shmem_key_line_prefix/m; $port_holder->stop; } + $port = $lock_port + 1; } # Node setup. @@ -57,6 +67,8 @@ sub init_start my $ret = PostgresNode->get_new_node($name, port => $port, own_host => 1); defined($port) or $port = $ret->port; # same port for all nodes $ret->init; + # Limit semaphore consumption, since we run several nodes concurrently. + $ret->append_conf('postgresql.conf', 'max_connections = 5'); $ret->start; log_ipcs(); return $ret; @@ -112,12 +124,22 @@ $gnat->kill9; unlink($gnat->data_dir . '/postmaster.pid'); $gnat->rotate_logfile; # on Windows, can't open old log for writing log_ipcs(); -# Reject ordinary startup. -ok(!$gnat->start(fail_ok => 1), 'live query blocks restart'); -like( - slurp_file($gnat->logfile), - qr/pre-existing shared memory block/, - 'detected live backend via shared memory'); +# Reject ordinary startup. Retry for the same reasons poll_start() does. +my $pre_existing_msg = qr/pre-existing shared memory block/; +{ + my $max_attempts = 180 * 10; # Retry every 0.1s for at least 180s. + my $attempts = 0; + while ($attempts < $max_attempts) + { + last + if $gnat->start(fail_ok => 1) + || slurp_file($gnat->logfile) =~ $pre_existing_msg; + usleep(100_000); + $attempts++; + } +} +like(slurp_file($gnat->logfile), + $pre_existing_msg, 'detected live backend via shared memory'); # Reject single-user startup. my $single_stderr; ok( !run_log( @@ -125,9 +147,7 @@ ok( !run_log( '<', \('SELECT 1 + 1'), '2>', \$single_stderr), 'live query blocks --single'); print STDERR $single_stderr; -like( - $single_stderr, - qr/pre-existing shared memory block/, +like($single_stderr, $pre_existing_msg, 'single-user mode detected live backend via shared memory'); log_ipcs(); # Fail to reject startup if shm key N has become available and we crash while @@ -151,8 +171,9 @@ $port_holder->stop if $port_holder; log_ipcs(); -# When postmaster children are slow to exit after postmaster death, we may -# need retries to start a new postmaster. +# When the kernel is slow to deliver SIGKILL, a postmaster child is slow to +# exit in response to SIGQUIT, or a postmaster child is slow to exit after +# postmaster death, we may need retries to start a new postmaster. sub poll_start { my ($node) = @_;