Checks in RegisterBackgroundWorker.()

The first patch on my "Refactoring backend fork+exec code" thread [0] 
changes the allocations of BackgroundWorkerList from plain malloc() to 
MemoryContextAlloc(PostmasterContext). However, that actually caused a 
segfault in worker_spi tests in EXEC_BACKEND mode.

BackgroundWorkerList is a postmaster-private data structure and should 
not be accessed in backends. That assumption failed in 
RegisterBackgroundWorker(). When you put worker_spi in 
shared_preload_libraries, its _PG_init() function calls 
RegisterBackgroundWorker(), as expected. But in EXEC_BACKEND mode, the 
library is loaded *again* in each backend process, and each of those 
loads also call RegisterBackgroundWorker(). It's too late to correctly 
register any static background workers at that stage, but 
RegisterBackgroundWorker() still goes through the motions and adds the 
element to BackgroundWorkerList. If you change the malloc() to 
MemoryContextAlloc(PostmasterContext), it segfaults because 
PostmasterContext == NULL in a backend process.

In summary, RegisterBackgroundWorker() is doing some questionable and 
useless work, when a shared preload library is loaded to a backend 
process in EXEC_BACKEND mode.

Attached patches:

1. Tighten/clarify those checks. See also commit message for details.
2. The patch from the other thread to switch to 
MemoryContextAlloc(PostmasterContext)
3. A fix for a highly misleading comment in the same file.

Any comments?

[0] 
https://www.postgresql.org/message-id/flat/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef%40iki.fi

P.S. In addition to those, I also considered these changes but didn't 
implement them yet:

- Change RegisterBackgroundWorker() to return true/false to indicate 
whether the registration succeeded. Currently, the caller has no way of 
knowing. In many cases, I think even an ERROR and refusing to start up 
the server would be appropriate. But at least we should let the caller 
know and decide.

- Add "Assert(am_postmaster)" assertions to the functions in bgworker.c 
that are only supposed to be called in postmaster process. The functions 
have good explicit comments on that, but wouldn't hurt to also assert. 
(There is no 'am_postmaster' flag, the equivalent is actually 
(IsPostmasterEnvironment && !IsUnderPostmaster), but perhaps we should 
define a macro or flag for that)

-- 
Heikki Linnakangas
Neon (https://neon.tech)