Re: [PATCH 1/1] Initial mach based shared memory support.

On Mon, Jan 18, 2021 at 11:12 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> James Hilliard <james(dot)hilliard1(at)gmail(dot)com> writes:
> > from my understanding due to mach semantics a number of the sanity checks
> > we are doing for sysv shmem are probably unnecessary when using mach
> > API's due to the mach port task bindings(mach_task_self()) effectively
> > ensuring our maps are already task bound and won't conflict with other tasks.
>
> Really? If so, this whole patch is pretty much dead in the water,
> because the fact that sysv shmem is globally accessible is exactly
> why we use it (well, that and the fact that you can find out how many
> processes are attached to it). It's been a long time since we cared
> about sysv shmem as a source of shared storage. What we really use
> it for is as a form of mutual exclusion, i.e. being able to detect
> whether any live children remain from a dead postmaster. That is,
> PGSharedMemoryIsInUse is not some minor ancillary check, it's the main
> reason this module exists at all. If POSIX-style mmap'd shmem had the
> same capability we'd likely have dropped sysv altogether long ago.
Oh, I had figured the mutual exclusion check was just to ensure that
one didn't accidentally overlap an existing shared memory map.

If it's just an additional sanity check maybe we should make it non-fatal for
ENOSPC returns as it is impossible to increase the sysv shm segment limits
on OSX from my understanding.

But there should be a way to do this with mach, I see a few options, the task
bound map behavior from my understanding is what you get when using
mach_task_self(), however you can also look up a task using pid_for_task()
from my understanding(there may be security checks here that could cause
issues). It also may make sense to write the postmaster task ID and use that
instead of the PID. I'll try and rework this.

By the way is PGSharedMemoryIsInUse only using the pid(id2) for looking up
the shmem segment key or is something else needed? I noticed there is an
unused id1 variable as well that gets passed to it.
>
> I've occasionally wondered if we should take another look at file locking
> as a mechanism for ensuring only one postmaster+children process group
> can access a data directory. Back in the day it was too untrustworthy,
> but maybe that has changed.
Yeah, there's probably some ugly edge cases there still.
>
> regards, tom lane