On 08/07/2026 11:47, Jelte Fennema-Nio wrote:
On Wed, Jul 8, 2026, 01:41 Thomas Munro <[email protected]> wrote:
On Tue, Jul 7, 2026 at 9:23 AM Heikki Linnakangas <[email protected]>
wrote:
> In this case, though, I think all we need is a "volatile sigatomic_t"
> flag. Sending the query cancellation over the network surely acts as a
> full compiler and memory barrier in the cancelling thread. And
similarly
> receiving the error message from the network acts as a full barrier in
> the other threads that might receive the cancellation error from the
> backend.
You're right. So basically Jelte's patch, except it doesn't need the
Win32 atomics stuff, just volatile, and a comment to explain that
assumption. (Then some later version could use an explicit barrier
instead of a comment, I guess, just to be clearer.)
Without any synchronization primitives there's still room for a
data-race, right?
The network operations on the cancel thread (to send cancellation) and
on the other thread (to receive the error messages from the server)
should act as reliable memory barriers.
But I guess in practice that doesn't matter for the messages we
actually care about.
Also true.
Attached is a patch with that change, altough I'm using "volatile
bool" instead of "volatile sigatomic_t" since there are no signal
handlers involved here.
I also added !is_cancel_in_progress() checks in a few more places to
silence the places that Bryan had found.
Ok, committed, thank you!
To be clear: putting all of these threads together I'd like to remove
this function again completely. Instead I'd like this to use
CancelRequested for this, but that requires my PQblockingCancel patchset
to be merged first. And then I'd like to make CancelRequested a C11
atomic_flag instead of a volatile.
Ack. I'm getting a little confused by all the interdependent patches
flying around. But as long as we keep grinding, committing what we can
one patch at a time and rebasing all remaining ones, we'll be done
eventually :-).
- Heikki
