Here's a new version of the first patch. In the previous version, I
added the pid cancellation key to pmsignal.c, but on second thoughts, I
think procsignal.c is a better place. The ProcSignal array already
contains the pid, we just need to add the cancellation key there.
This first patch just refactors the current code, without changing the
protocol or length of the cancellation key. I'd like to get this
reviewed and committed first, and get back to the protocol changes after
that.
We currently don't do any locking on the ProcSignal array. For query
cancellations, that's good because a query cancel packet is processed
without having a PGPROC entry, so we cannot take LWLocks. We could use
spinlocks though. In this patch, I used memory barriers to ensure that
we load/store the pid and the cancellation key in a sensible order, so
that you cannot e.g. send a cancellation signal to a backend that's just
starting up and hasn't advertised its cancellation key in ProcSignal
yet. But I think this might be simpler and less error-prone by just
adding a spinlock to each ProcSignal slot. That would also fix the
existing race condition where we might set the pss_signalFlags flag for
a slot, when the process concurrently terminates and the slot is reused
for a different process. Because of that, we currently have this caveat:
"... all the signals are such that no harm is done if they're mistakenly
fired". With a spinlock, we could eliminate that race.
Thoughts?
--
Heikki Linnakangas
Neon (https://neon.tech)