Message-ID: <64ff0801-e6fb-4c4f-8bb8-67391e888860@iki.fi>
Date: Wed, 8 Jul 2026 18:49:47 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Can we get rid of TerminateThread() in pg_dump?
To: Jelte Fennema-Nio <postgres@jeltef.nl>,
 Thomas Munro <thomas.munro@gmail.com>
Cc: Bryan Green <dbryan.green@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
References: 
 <CA+hUKGJgO=o-vLFahGdR2WesuX3h1-0j=a8z72fChc-MG1Hveg@mail.gmail.com>
 <56821e0d-bfd5-4619-b13f-12e28c78289b@iki.fi>
 <faea6e93-32ab-46ad-8caa-4f502cb3e63c@gmail.com>
 <CA+hUKGL0ZeiO0ND+ksAi5inRY98GEGtyM4t8CVhF1mZaAqYjVA@mail.gmail.com>
 <DJPQS3FYSD4U.3DBTXA6U8IQ0Q@jeltef.nl>
 <CA+hUKGJ3WNcqtj0xt8k9cnBRTEjAPkT4QrQrAbEgO6T7fUfPXA@mail.gmail.com>
 <5e4a28ec-d5e1-4817-889c-b8aefe5f48ed@iki.fi>
 <400c93d2-0b81-4a0b-ac60-0fd6512d5b5d@iki.fi>
 <CA+hUKG+4ULA_-bW0Fws8SoJGWgS49d76kB264A7yC8pH=k_joQ@mail.gmail.com>
 <DJT25I9FT73D.2I002MPUQ1ZM0@jeltef.nl>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <DJT25I9FT73D.2I002MPUQ1ZM0@jeltef.nl>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/64ff0801-e6fb-4c4f-8bb8-67391e888860%40iki.fi>
Precedence: bulk

On 08/07/2026 11:47, Jelte Fennema-Nio wrote:
> On Wed, Jul 8, 2026, 01:41 Thomas Munro <thomas.munro@gmail.com> wrote:
> 
>> On Tue, Jul 7, 2026 at 9:23 AM Heikki Linnakangas <hlinnaka@iki.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