Re: Don't use the deprecated and insecure PQcancel in our frontend tools anymore

Attached a v8 based on this feedback, and I think much better than the
previous version. The first 2 commits are simplifications of the current
logic that I think make sense to merge even without the rest of the
patchset.

On Tue, 7 Apr 2026 at 02:18, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> True, it can get messy fast. Hmm, perhaps you could check
> PQcancelStatus() and only cancel the cancellation if it hasn't sent the
> CancelRequest packet yet.

I kinda don't wanna make this logic even more complicated.

> The more I look into the cancel handling in psql and other client
> programs, the more I want to gouge my eyes out.

Same here...

> It was pretty messy
> before this patch, too, and now also we have an extra thread to worry about.

Most of the complexity of the extra thread was already there for
Windows. Making Windows and Unix behave more similarly actually reduces
the overall complexity, IMO.

> Perhaps is should be documented that the callback is called first.

Done

> The cancel handling in wait_on_slots() in parallel_slot.c is surprising
> in a different way. It already uses async libpq calls and has a select()
> loop, but it still relies on the signal handler to do the cancellation.
> And it arbitrarily PQcancel()s only one of the connections it waits on.

Addressed this in 0002

> The comments you added in your patches help a lot, explaining the three
> different ways that cancellation can be handled, thanks for that.

Yeah, I think

> psql has a global variable, 'cancel_pressed', which is set in the signal
> handler callback. Is it redundant with 'CancelRequested' that's also set
> in the signal handler?

I did some spelunking, and yes it is redundant. Fixed in 0001

> Some ideas on how to make this less confusing:
>
> - Change pg_dump to use threads on Unix too.

I think that would be a good follow-up, but I don't think it makes sense
as part of this patchset. The only thing that would make more aligned
between OSes with respect to this patch is the worker process vs thread
shutdown.

> - Extract the cancel_pipe stuff into a helper function, and reuse it in
> pg_dump and in wait_on_slots()

This is what I did in the end and I think it improved the patchset a lot.

> Another more radical idea: Could we move this functionality to libpq
> itself? Imagine that we had a function like PQrequestCancel(PGconn
> *conn) that just set a flag on the PGconn object to indicate that
> cancellation has been requested. When that flag is set, all blocking
> calls like PQexec() on the original connection drive the cancellation
> connection and sending the cancel request

I think it's an interesting idea (and I'm evaluating it a bit more in
the background), but it's quite a big addition to libpq and I'm not sure
it pulls its own weight. It also doesn't fit well with the pg_dump
approach: "Fire off the cancel requests, then kill the process." By
moving sending the cancel request to the blocking call, there's no way
to bail out after the cancel is sent but before the blocking call
returns (i.e. a query not responding to the cancel would then block the
pg_dump shutdown).