| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
| Cc: | Bryan Green <dbryan(dot)green(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
| Subject: | Re: Can we get rid of TerminateThread() in pg_dump? |
| Date: | 2026-07-08 15:49:47 |
| Message-ID: | 64ff0801-e6fb-4c4f-8bb8-67391e888860@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 08/07/2026 11:47, Jelte Fennema-Nio wrote:
> On Wed, Jul 8, 2026, 01:41 Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
>> On Tue, Jul 7, 2026 at 9:23 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
>> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-07-08 16:03:23 | pg_hosts: Add pg_hosts_file_rules() |
| Previous Message | Matthias van de Meent | 2026-07-08 15:49:16 | Re: Increased SECURITY RISCS from omitting some compikler options when building PG with meson; e.g. -fcf-protection=full |