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 <[email protected]> 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).
From 51382cf4407f9b924b649c97c8e59b3024c05925 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Tue, 7 Apr 2026 18:24:01 +0200 Subject: [PATCH v8 1/4] psql: Replace cancel_pressed with CancelRequested In a4fd3aa7 src/fe_utils/cancel.c was introduced which added the CancelRequested variable. This new variable was used in psql to replace the cancel_pressed variable, but that quickly got reverted in 5d43c3c54. The reason was that cancel_pressed had not fully replaced by CancelRequested everywhere in the code, and the mixed usage caused issues (e.g. with \watch). This now does that original refactor correctly by using CancelRequested everywhere and completely getting rid of cancel_pressed. 5d43c3c54 mentions the --single-step flag as something that required further analysis. I tried --single-step with and without this commit, and Ctrl+C behaves the same in both. I also cannot think of a reason why it would behave any differently. The only slight behavioral change I could think of was that cancel_pressed was set after psql's longjump, and CancelRequested is set before. But since CancelRequested is now set to false after the longjump there's no practical difference caused by that. --- src/bin/psql/command.c | 10 ++-- src/bin/psql/common.c | 21 ++++---- src/bin/psql/describe.c | 9 ++-- src/bin/psql/mainloop.c | 7 +-- src/bin/psql/variables.c | 3 +- src/fe_utils/print.c | 101 +++++++++++++++++------------------ src/include/fe_utils/print.h | 2 - 7 files changed, 74 insertions(+), 79 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 01b8f11aadd..85a61d5990f 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4392,7 +4392,7 @@ wait_until_connected(PGconn *conn) * On every iteration of the connection sequence, let's check if the * user has requested a cancellation. */ - if (cancel_pressed) + if (CancelRequested) break; /* @@ -4404,7 +4404,7 @@ wait_until_connected(PGconn *conn) break; /* - * If the user sends SIGINT between the cancel_pressed check, and + * If the user sends SIGINT between the CancelRequested check, and * polling of the socket, it will not be recognized. Instead, we will * just wait until the next step in the connection sequence or * forever, which might require users to send SIGTERM or SIGQUIT. @@ -4415,7 +4415,7 @@ wait_until_connected(PGconn *conn) * The self-pipe trick requires a bit of code to setup. pselect(2) and * ppoll(2) are not on all the platforms we support. The simplest * solution happens to just be adding a timeout, so let's wait for 1 - * second and check cancel_pressed again. + * second and check CancelRequested again. */ end_time = PQgetCurrentTimeUSec() + 1000000; rc = PQsocketPoll(sock, forRead, !forRead, end_time); @@ -6082,7 +6082,7 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows) long s = Min(i, 1000L); pg_usleep(s * 1000L); - if (cancel_pressed) + if (CancelRequested) { done = true; break; @@ -6092,7 +6092,7 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows) #else /* sigwait() will handle SIGINT. */ sigprocmask(SIG_BLOCK, &sigint, NULL); - if (cancel_pressed) + if (CancelRequested) done = true; /* Wait for SIGINT, SIGCHLD or SIGALRM. */ diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 10078f24532..660f14559f5 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -292,7 +292,7 @@ NoticeProcessor(void *arg, const char *message) * * SIGINT is supposed to abort all long-running psql operations, not only * database queries. In most places, this is accomplished by checking - * cancel_pressed during long-running loops. However, that won't work when + * CancelRequested during long-running loops. However, that won't work when * blocked on user input (in readline() or fgets()). In those places, we * set sigint_interrupt_enabled true while blocked, instructing the signal * catcher to longjmp through sigint_interrupt_jmp. We assume readline and @@ -316,9 +316,6 @@ psql_cancel_callback(void) siglongjmp(sigint_interrupt_jmp, 1); } #endif - - /* else, set cancel flag to stop any long-running loops */ - cancel_pressed = true; } void @@ -881,8 +878,8 @@ ExecQueryTuples(const PGresult *result) { const char *query = PQgetvalue(result, r, c); - /* Abandon execution if cancel_pressed */ - if (cancel_pressed) + /* Abandon execution if CancelRequested */ + if (CancelRequested) goto loop_exit; /* @@ -1146,7 +1143,7 @@ SendQuery(const char *query) if (fgets(buf, sizeof(buf), stdin) != NULL) if (buf[0] == 'x') goto sendquery_cleanup; - if (cancel_pressed) + if (CancelRequested) goto sendquery_cleanup; } else if (pset.echo == PSQL_ECHO_QUERIES) @@ -1768,7 +1765,7 @@ ExecQueryAndProcessResults(const char *query, * consumed. The user's intention, though, is to cancel the entire watch * process, so detect a sent cancellation request and exit in this case. */ - if (is_watch && cancel_pressed) + if (is_watch && CancelRequested) { ClearOrSaveAllResults(); return 0; @@ -1986,7 +1983,7 @@ ExecQueryAndProcessResults(const char *query, * use of chunking for all cases in which PrintQueryResult * would send the result to someplace other than printQuery. */ - if (success && !flush_error && !cancel_pressed) + if (success && !flush_error && !CancelRequested) { printQuery(result, &my_popt, tuples_fout, is_pager, pset.logfile); flush_error = fflush(tuples_fout); @@ -2013,7 +2010,7 @@ ExecQueryAndProcessResults(const char *query, Assert(PQntuples(result) == 0); /* Display the footer using the empty result */ - if (success && !flush_error && !cancel_pressed) + if (success && !flush_error && !CancelRequested) { my_popt.topt.stop_table = true; printQuery(result, &my_popt, tuples_fout, is_pager, pset.logfile); @@ -2172,7 +2169,7 @@ ExecQueryAndProcessResults(const char *query, ClearOrSaveResult(result); result = next_result; - if (cancel_pressed && PQpipelineStatus(pset.db) == PQ_PIPELINE_OFF) + if (CancelRequested && PQpipelineStatus(pset.db) == PQ_PIPELINE_OFF) { /* * Outside of a pipeline, drop the next result, as well as any @@ -2229,7 +2226,7 @@ ExecQueryAndProcessResults(const char *query, if (!CheckConnection()) return -1; - if (cancel_pressed || return_early) + if (CancelRequested || return_early) return 0; return success ? 1 : -1; diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index af3935b0078..be4630692b4 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -32,6 +32,7 @@ #include "common.h" #include "common/logging.h" #include "describe.h" +#include "fe_utils/cancel.h" #include "fe_utils/mbprint.h" #include "fe_utils/print.h" #include "fe_utils/string_utils.h" @@ -1566,7 +1567,7 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem) PQclear(res); return false; } - if (cancel_pressed) + if (CancelRequested) { PQclear(res); return false; @@ -5701,7 +5702,7 @@ listTSParsersVerbose(const char *pattern) return false; } - if (cancel_pressed) + if (CancelRequested) { PQclear(res); return false; @@ -6091,7 +6092,7 @@ listTSConfigsVerbose(const char *pattern) return false; } - if (cancel_pressed) + if (CancelRequested) { PQclear(res); return false; @@ -6570,7 +6571,7 @@ listExtensionContents(const char *pattern) PQclear(res); return false; } - if (cancel_pressed) + if (CancelRequested) { PQclear(res); return false; diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c index e9abda07161..6f5c0a77c03 100644 --- a/src/bin/psql/mainloop.c +++ b/src/bin/psql/mainloop.c @@ -10,6 +10,7 @@ #include "command.h" #include "common.h" #include "common/logging.h" +#include "fe_utils/cancel.h" #include "input.h" #include "mainloop.h" #include "mb/pg_wchar.h" @@ -85,7 +86,7 @@ MainLoop(FILE *source) /* * Clean up after a previous Control-C */ - if (cancel_pressed) + if (CancelRequested) { if (!pset.cur_cmd_interactive) { @@ -96,7 +97,7 @@ MainLoop(FILE *source) break; } - cancel_pressed = false; + CancelRequested = false; } /* @@ -118,7 +119,7 @@ MainLoop(FILE *source) prompt_status = PROMPT_READY; need_redisplay = false; pset.stmt_lineno = 1; - cancel_pressed = false; + CancelRequested = false; if (pset.cur_cmd_interactive) { diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 8060f2959cc..71eeafd039f 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -11,6 +11,7 @@ #include "common.h" #include "common/logging.h" +#include "fe_utils/cancel.h" #include "variables.h" /* @@ -265,7 +266,7 @@ PrintVariables(VariableSpace space) { if (ptr->value) printf("%s = '%s'\n", ptr->name, ptr->value); - if (cancel_pressed) + if (CancelRequested) break; } } diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index bfbaf094d7e..c9f9f23f6f5 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -4,7 +4,7 @@ * * This file used to be part of psql, but now it's separated out to allow * other frontend programs to use it. Because the printing code needs - * access to the cancel_pressed flag as well as SIGPIPE trapping and + * access to the CancelRequested flag as well as SIGPIPE trapping and * pager open/close functions, all that stuff came with it. * * @@ -30,6 +30,7 @@ #endif #include "catalog/pg_type_d.h" +#include "fe_utils/cancel.h" #include "fe_utils/mbprint.h" #include "fe_utils/print.h" @@ -39,13 +40,9 @@ #endif /* - * If the calling program doesn't have any mechanism for setting - * cancel_pressed, it will have no effect. - * - * Note: print.c's general strategy for when to check cancel_pressed is to do + * Note: print.c's general strategy for when to check CancelRequested is to do * so at completion of each row of output. */ -volatile sig_atomic_t cancel_pressed = false; static bool always_ignore_sigpipe = false; @@ -442,7 +439,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout) const char *const *ptr; bool need_recordsep = false; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -477,7 +474,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout) { print_separator(cont->opt->recordSep, fout); need_recordsep = false; - if (cancel_pressed) + if (CancelRequested) break; } fputs(*ptr, fout); @@ -493,7 +490,7 @@ print_unaligned_text(const printTableContent *cont, FILE *fout) { printTableFooter *footers = footers_with_default(cont); - if (!opt_tuples_only && footers != NULL && !cancel_pressed) + if (!opt_tuples_only && footers != NULL && !CancelRequested) { printTableFooter *f; @@ -533,7 +530,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout) const char *const *ptr; bool need_recordsep = false; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -558,7 +555,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout) print_separator(cont->opt->recordSep, fout); print_separator(cont->opt->recordSep, fout); need_recordsep = false; - if (cancel_pressed) + if (CancelRequested) break; } @@ -575,7 +572,7 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout) if (cont->opt->stop_table) { /* print footers */ - if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed) + if (!opt_tuples_only && cont->footers != NULL && !CancelRequested) { printTableFooter *f; @@ -683,7 +680,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) int output_columns = 0; /* Width of interactive console */ bool is_local_pager = false; - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 2) @@ -1004,7 +1001,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) { bool more_lines; - if (cancel_pressed) + if (CancelRequested) break; /* @@ -1160,12 +1157,12 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager) { printTableFooter *footers = footers_with_default(cont); - if (opt_border == 2 && !cancel_pressed) + if (opt_border == 2 && !CancelRequested) _print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_BOTTOM, format, fout); /* print footers */ - if (footers && !opt_tuples_only && !cancel_pressed) + if (footers && !opt_tuples_only && !CancelRequested) { printTableFooter *f; @@ -1325,7 +1322,7 @@ print_aligned_vertical(const printTableContent *cont, dmultiline = false; int output_columns = 0; /* Width of interactive console */ - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 2) @@ -1341,7 +1338,7 @@ print_aligned_vertical(const printTableContent *cont, { printTableFooter *footers = footers_with_default(cont); - if (!opt_tuples_only && !cancel_pressed && footers) + if (!opt_tuples_only && !CancelRequested && footers) { printTableFooter *f; @@ -1580,7 +1577,7 @@ print_aligned_vertical(const printTableContent *cont, offset, chars_to_output; - if (cancel_pressed) + if (CancelRequested) break; if (i == 0) @@ -1789,12 +1786,12 @@ print_aligned_vertical(const printTableContent *cont, if (cont->opt->stop_table) { - if (opt_border == 2 && !cancel_pressed) + if (opt_border == 2 && !CancelRequested) print_aligned_vertical_line(cont->opt, 0, hwidth, dwidth, output_columns, PRINT_RULE_BOTTOM, fout); /* print footers */ - if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed) + if (!opt_tuples_only && cont->footers != NULL && !CancelRequested) { printTableFooter *f; @@ -1868,7 +1865,7 @@ print_csv_text(const printTableContent *cont, FILE *fout) const char *const *ptr; int i; - if (cancel_pressed) + if (CancelRequested) return; /* @@ -1911,7 +1908,7 @@ print_csv_vertical(const printTableContent *cont, FILE *fout) /* print records */ for (i = 0, ptr = cont->cells; *ptr; i++, ptr++) { - if (cancel_pressed) + if (CancelRequested) return; /* print name of column */ @@ -1984,7 +1981,7 @@ print_html_text(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -2021,7 +2018,7 @@ print_html_text(const printTableContent *cont, FILE *fout) { if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; fputs(" <tr valign=\"top\">\n", fout); } @@ -2046,7 +2043,7 @@ print_html_text(const printTableContent *cont, FILE *fout) fputs("</table>\n", fout); /* print footers */ - if (!opt_tuples_only && footers != NULL && !cancel_pressed) + if (!opt_tuples_only && footers != NULL && !CancelRequested) { printTableFooter *f; @@ -2074,7 +2071,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -2098,7 +2095,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout) { if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; if (!opt_tuples_only) fprintf(fout, @@ -2127,7 +2124,7 @@ print_html_vertical(const printTableContent *cont, FILE *fout) fputs("</table>\n", fout); /* print footers */ - if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed) + if (!opt_tuples_only && cont->footers != NULL && !CancelRequested) { printTableFooter *f; @@ -2176,7 +2173,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -2235,7 +2232,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout) { if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; } @@ -2263,7 +2260,7 @@ print_asciidoc_text(const printTableContent *cont, FILE *fout) printTableFooter *footers = footers_with_default(cont); /* print footers */ - if (!opt_tuples_only && footers != NULL && !cancel_pressed) + if (!opt_tuples_only && footers != NULL && !CancelRequested) { printTableFooter *f; @@ -2287,7 +2284,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->start_table) @@ -2326,7 +2323,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout) { if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; if (!opt_tuples_only) fprintf(fout, @@ -2353,7 +2350,7 @@ print_asciidoc_vertical(const printTableContent *cont, FILE *fout) if (cont->opt->stop_table) { /* print footers */ - if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed) + if (!opt_tuples_only && cont->footers != NULL && !CancelRequested) { printTableFooter *f; @@ -2444,7 +2441,7 @@ print_latex_text(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 3) @@ -2505,7 +2502,7 @@ print_latex_text(const printTableContent *cont, FILE *fout) fputs(" \\\\\n", fout); if (opt_border == 3) fputs("\\hline\n", fout); - if (cancel_pressed) + if (CancelRequested) break; } else @@ -2522,7 +2519,7 @@ print_latex_text(const printTableContent *cont, FILE *fout) fputs("\\end{tabular}\n\n\\noindent ", fout); /* print footers */ - if (footers && !opt_tuples_only && !cancel_pressed) + if (footers && !opt_tuples_only && !CancelRequested) { printTableFooter *f; @@ -2554,7 +2551,7 @@ print_latex_longtable_text(const printTableContent *cont, FILE *fout) const char *last_opt_table_attr_char = NULL; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 3) @@ -2690,7 +2687,7 @@ print_latex_longtable_text(const printTableContent *cont, FILE *fout) if (opt_border == 3) fputs(" \\hline\n", fout); } - if (cancel_pressed) + if (CancelRequested) break; } @@ -2708,7 +2705,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 2) @@ -2741,7 +2738,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout) /* new record */ if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; if (!opt_tuples_only) { @@ -2771,7 +2768,7 @@ print_latex_vertical(const printTableContent *cont, FILE *fout) fputs("\\end{tabular}\n\n\\noindent ", fout); /* print footers */ - if (cont->footers && !opt_tuples_only && !cancel_pressed) + if (cont->footers && !opt_tuples_only && !CancelRequested) { printTableFooter *f; @@ -2817,7 +2814,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout) unsigned int i; const char *const *ptr; - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 2) @@ -2871,7 +2868,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout) if ((i + 1) % cont->ncolumns == 0) { fputc('\n', fout); - if (cancel_pressed) + if (CancelRequested) break; } else @@ -2885,7 +2882,7 @@ print_troff_ms_text(const printTableContent *cont, FILE *fout) fputs(".TE\n.DS L\n", fout); /* print footers */ - if (footers && !opt_tuples_only && !cancel_pressed) + if (footers && !opt_tuples_only && !CancelRequested) { printTableFooter *f; @@ -2911,7 +2908,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout) const char *const *ptr; unsigned short current_format = 0; /* 0=none, 1=header, 2=body */ - if (cancel_pressed) + if (CancelRequested) return; if (opt_border > 2) @@ -2947,7 +2944,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout) /* new record */ if (i % cont->ncolumns == 0) { - if (cancel_pressed) + if (CancelRequested) break; if (!opt_tuples_only) { @@ -2992,7 +2989,7 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout) fputs(".TE\n.DS L\n", fout); /* print footers */ - if (cont->footers && !opt_tuples_only && !cancel_pressed) + if (cont->footers && !opt_tuples_only && !CancelRequested) { printTableFooter *f; @@ -3170,7 +3167,7 @@ ClosePager(FILE *pagerpipe) * pager quit as a result of the SIGINT, this message won't go * anywhere ... */ - if (cancel_pressed) + if (CancelRequested) fprintf(pagerpipe, _("Interrupted\n")); pclose(pagerpipe); @@ -3639,7 +3636,7 @@ printTable(const printTableContent *cont, { bool is_local_pager = false; - if (cancel_pressed) + if (CancelRequested) return; if (cont->opt->format == PRINT_NOTHING) @@ -3748,7 +3745,7 @@ printQuery(const PGresult *result, const printQueryOpt *opt, r, c; - if (cancel_pressed) + if (CancelRequested) return; printTableInit(&cont, &opt->topt, opt->title, diff --git a/src/include/fe_utils/print.h b/src/include/fe_utils/print.h index 94f6a593619..23097ba853a 100644 --- a/src/include/fe_utils/print.h +++ b/src/include/fe_utils/print.h @@ -195,8 +195,6 @@ typedef struct printQueryOpt } printQueryOpt; -extern PGDLLIMPORT volatile sig_atomic_t cancel_pressed; - extern PGDLLIMPORT const printTextFormat pg_asciiformat; extern PGDLLIMPORT const printTextFormat pg_asciiformat_old; extern PGDLLIMPORT printTextFormat pg_utf8format; /* ideally would be const, base-commit: e42d4a1f3dc59420be796883404020cb41ddd05e -- 2.54.0
From e6bb82c07ac74e8f7a528a57b96e7cffee6d613a Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Tue, 7 Apr 2026 19:54:13 +0200 Subject: [PATCH v8 2/4] fe_utils: Simplify cancel logic in wait_on_slots This removes the SetCancelConn call in wait_on_slots. This call was confusing because it was only called for a single connection of all the connections that the select_loop would wait on. Turns out that it can be be made completely redundant by moving the check for CancelRequested before the check for EINTR. --- src/fe_utils/parallel_slot.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/fe_utils/parallel_slot.c b/src/fe_utils/parallel_slot.c index fb9e6cc4ec1..94d13cc499a 100644 --- a/src/fe_utils/parallel_slot.c +++ b/src/fe_utils/parallel_slot.c @@ -103,6 +103,9 @@ select_loop(int maxFd, fd_set *workerset) *workerset = saveSet; i = select(maxFd + 1, workerset, NULL, NULL, tvp); + if (CancelRequested) + return -1; + #ifdef WIN32 if (i == SOCKET_ERROR) { @@ -115,7 +118,7 @@ select_loop(int maxFd, fd_set *workerset) if (i < 0 && errno == EINTR) continue; /* ignore this */ - if (i < 0 || CancelRequested) + if (i < 0) return -1; /* but not this */ if (i == 0) continue; /* timeout (Win32 only) */ @@ -197,8 +200,7 @@ wait_on_slots(ParallelSlotArray *sa) { int i; fd_set slotset; - int maxFd = 0; - PGconn *cancelconn = NULL; + int maxFd = -1; /* We must reconstruct the fd_set for each call to select_loop */ FD_ZERO(&slotset); @@ -219,10 +221,6 @@ wait_on_slots(ParallelSlotArray *sa) if (sock < 0) continue; - /* Keep track of the first valid connection we see. */ - if (cancelconn == NULL) - cancelconn = sa->slots[i].connection; - FD_SET(sock, &slotset); if (sock > maxFd) maxFd = sock; @@ -232,12 +230,10 @@ wait_on_slots(ParallelSlotArray *sa) * If we get this far with no valid connections, processing cannot * continue. */ - if (cancelconn == NULL) + if (maxFd < 0) return false; - SetCancelConn(cancelconn); i = select_loop(maxFd, &slotset); - ResetCancelConn(); /* failure? */ if (i < 0) -- 2.54.0
From 0a5d0c56c72637c6c223811a752da3e2b387725f Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Sat, 13 Dec 2025 18:18:13 +0100 Subject: [PATCH v8 3/4] Move Windows pthread compatibility functions to src/port This is in preparation of a follow-up commit which will start to use these functions in more places than just libpq. --- configure.ac | 1 + src/interfaces/libpq/Makefile | 1 - src/interfaces/libpq/meson.build | 2 +- src/port/meson.build | 1 + src/{interfaces/libpq => port}/pthread-win32.c | 4 ++-- 5 files changed, 5 insertions(+), 4 deletions(-) rename src/{interfaces/libpq => port}/pthread-win32.c (94%) diff --git a/configure.ac b/configure.ac index 61cee42daa7..0ce03782733 100644 --- a/configure.ac +++ b/configure.ac @@ -1961,6 +1961,7 @@ if test "$PORTNAME" = "win32"; then AC_LIBOBJ(dirmod) AC_LIBOBJ(kill) AC_LIBOBJ(open) + AC_LIBOBJ(pthread-win32) AC_LIBOBJ(system) AC_LIBOBJ(win32common) AC_LIBOBJ(win32dlopen) diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile index 0963995eed4..d6cfb00d655 100644 --- a/src/interfaces/libpq/Makefile +++ b/src/interfaces/libpq/Makefile @@ -75,7 +75,6 @@ endif ifeq ($(PORTNAME), win32) OBJS += \ - pthread-win32.o \ win32.o endif diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build index b0ae72167a1..b949780b85b 100644 --- a/src/interfaces/libpq/meson.build +++ b/src/interfaces/libpq/meson.build @@ -20,7 +20,7 @@ libpq_sources = files( libpq_so_sources = [] # for shared lib, in addition to the above if host_system == 'windows' - libpq_sources += files('pthread-win32.c', 'win32.c') + libpq_sources += files('win32.c') libpq_so_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ '--NAME', 'libpq', '--FILEDESC', 'PostgreSQL Access Library',]) diff --git a/src/port/meson.build b/src/port/meson.build index 922b3f64676..10833b13842 100644 --- a/src/port/meson.build +++ b/src/port/meson.build @@ -33,6 +33,7 @@ if host_system == 'windows' 'dirmod.c', 'kill.c', 'open.c', + 'pthread-win32.c', 'system.c', 'win32common.c', 'win32dlopen.c', diff --git a/src/interfaces/libpq/pthread-win32.c b/src/port/pthread-win32.c similarity index 94% rename from src/interfaces/libpq/pthread-win32.c rename to src/port/pthread-win32.c index cf66284f007..48d68b693a7 100644 --- a/src/interfaces/libpq/pthread-win32.c +++ b/src/port/pthread-win32.c @@ -5,12 +5,12 @@ * * Copyright (c) 2004-2026, PostgreSQL Global Development Group * IDENTIFICATION -* src/interfaces/libpq/pthread-win32.c +* src/port/pthread-win32.c * *------------------------------------------------------------------------- */ -#include "postgres_fe.h" +#include "c.h" #include "pthread-win32.h" -- 2.54.0
From 11621934ca15ab9b624302e7bd87bc3f1bd343b2 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Sat, 13 Dec 2025 13:05:50 +0100 Subject: [PATCH v8 4/4] Don't use deprecated and insecure PQcancel psql and other tools anymore All of our frontend tools that used our fe_utils to cancel queries, including psql, still used PQcancel to send cancel requests to the server. That function is insecure, because it does not use encryption to send the cancel request. This starts using the new cancellation APIs (introduced in 61461a300) for all these frontend tools. These APIs use the same encryption settings as the connection that's being cancelled. Since these APIs are not signal-safe this required a refactor to not send the cancel request in a signal handler anymore, but instead using a dedicated thread. Similar logic was already used for Windows anyway, so this has the benefit that it makes the cancel logic more uniform across our supported platforms. In the pg_dump code there's still quite a bit of behavioural difference though, because pg_dump is using threads for parallelism on Windows, but processes on Unixes. --- meson.build | 2 +- src/bin/pg_amcheck/pg_amcheck.c | 2 +- src/bin/pg_dump/Makefile | 2 +- src/bin/pg_dump/meson.build | 2 + src/bin/pg_dump/parallel.c | 343 ++++++++------------ src/bin/pg_dump/pg_backup_archiver.c | 2 +- src/bin/pg_dump/pg_backup_archiver.h | 8 +- src/bin/pg_dump/pg_backup_db.c | 7 +- src/bin/pgbench/pgbench.c | 2 +- src/bin/psql/common.c | 10 +- src/bin/scripts/clusterdb.c | 2 +- src/bin/scripts/reindexdb.c | 2 +- src/bin/scripts/vacuuming.c | 2 +- src/fe_utils/Makefile | 2 +- src/fe_utils/cancel.c | 469 ++++++++++++++++++++------- src/include/fe_utils/cancel.h | 15 +- 16 files changed, 520 insertions(+), 352 deletions(-) diff --git a/meson.build b/meson.build index 568e0e150bf..0ac0051a9b5 100644 --- a/meson.build +++ b/meson.build @@ -3650,7 +3650,7 @@ frontend_code = declare_dependency( include_directories: [postgres_inc], link_with: [fe_utils, common_static, pgport_static], sources: generated_headers_stamp, - dependencies: [os_deps, libintl], + dependencies: [os_deps, libintl, thread_dep], ) backend_both_deps += [ diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c index 09ba0596400..2728bcdb1aa 100644 --- a/src/bin/pg_amcheck/pg_amcheck.c +++ b/src/bin/pg_amcheck/pg_amcheck.c @@ -478,7 +478,7 @@ main(int argc, char *argv[]) cparams.dbname = NULL; cparams.override_dbname = NULL; - setup_cancel_handler(NULL); + setup_cancel_handler(NULL, NULL); /* choose the database for our initial connection */ if (opts.alldb) diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile index 79073b0a0ea..f76346c4f6c 100644 --- a/src/bin/pg_dump/Makefile +++ b/src/bin/pg_dump/Makefile @@ -21,7 +21,7 @@ export LZ4 export ZSTD export with_icu -override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) +override CPPFLAGS := -I$(libpq_srcdir) -I$(top_srcdir)/src/port $(CPPFLAGS) LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) OBJS = \ diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build index 79bd5036841..eb55d7a50cf 100644 --- a/src/bin/pg_dump/meson.build +++ b/src/bin/pg_dump/meson.build @@ -22,6 +22,8 @@ pg_dump_common_sources = files( pg_dump_common = static_library('libpgdump_common', pg_dump_common_sources, c_pch: pch_postgres_fe_h, + # port needs to be in include path due to pthread-win32.h + include_directories: ['../../port'], dependencies: [frontend_code, libpq, lz4, zlib, zstd], kwargs: internal_lib_args, ) diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c index 7e2e9f958ea..1d76e91fc48 100644 --- a/src/bin/pg_dump/parallel.c +++ b/src/bin/pg_dump/parallel.c @@ -60,6 +60,8 @@ #include <fcntl.h> #endif +#include "common/logging.h" +#include "fe_utils/cancel.h" #include "fe_utils/string_utils.h" #include "parallel.h" #include "pg_backup_utils.h" @@ -174,10 +176,6 @@ typedef struct DumpSignalInformation static volatile DumpSignalInformation signal_info; -#ifdef WIN32 -static CRITICAL_SECTION signal_info_lock; -#endif - /* * Write a simple string to stderr --- must be safe in a signal handler. * We ignore the write() result since there's not much we could do about it. @@ -209,6 +207,7 @@ static void WaitForTerminatingWorkers(ParallelState *pstate); static void set_cancel_handler(void); static void set_cancel_pstate(ParallelState *pstate); static void set_cancel_slot_archive(ParallelSlot *slot, ArchiveHandle *AH); +static void StopWorkers(void); static void RunWorker(ArchiveHandle *AH, ParallelSlot *slot); static int GetIdleWorker(ParallelState *pstate); static bool HasEveryWorkerTerminated(ParallelState *pstate); @@ -410,32 +409,9 @@ ShutdownWorkersHard(ParallelState *pstate) /* * Force early termination of any commands currently in progress. */ -#ifndef WIN32 - /* On non-Windows, send SIGTERM to each worker process. */ - for (i = 0; i < pstate->numWorkers; i++) - { - pid_t pid = pstate->parallelSlot[i].pid; - - if (pid != 0) - kill(pid, SIGTERM); - } -#else - - /* - * On Windows, send query cancels directly to the workers' backends. Use - * a critical section to ensure worker threads don't change state. - */ - EnterCriticalSection(&signal_info_lock); - for (i = 0; i < pstate->numWorkers; i++) - { - ArchiveHandle *AH = pstate->parallelSlot[i].AH; - char errbuf[1]; - - if (AH != NULL && AH->connCancel != NULL) - (void) PQcancel(AH->connCancel, errbuf, sizeof(errbuf)); - } - LeaveCriticalSection(&signal_info_lock); -#endif + LockCancelThread(); + StopWorkers(); + UnlockCancelThread(); /* Now wait for them to terminate. */ WaitForTerminatingWorkers(pstate); @@ -519,74 +495,80 @@ WaitForTerminatingWorkers(ParallelState *pstate) * could leave a SQL command (e.g., CREATE INDEX on a large table) running * for a long time. Instead, we try to send a cancel request and then die. * pg_dump probably doesn't really need this, but we might as well use it - * there too. Note that sending the cancel directly from the signal handler - * is safe because PQcancel() is written to make it so. + * there too. * - * In parallel operation on Unix, each process is responsible for canceling - * its own connection (this must be so because nobody else has access to it). - * Furthermore, the leader process should attempt to forward its signal to - * each child. In simple manual use of pg_dump/pg_restore, forwarding isn't - * needed because typing control-C at the console would deliver SIGINT to - * every member of the terminal process group --- but in other scenarios it - * might be that only the leader gets signaled. + * On Unix, the signal handler wakes up a dedicated cancel thread via a + * self-pipe, which then sends the cancel and calls _exit(). This thread also + * forwards the signal to each child so they can also cancel their queries. In + * simple manual use of pg_dump/pg_restore, forwarding isn't needed because + * typing control-C at the console would deliver SIGINT to every member of the + * terminal process group --- but in other scenarios it might be that only the + * leader gets signaled. * * On Windows, the cancel handler runs in a separate thread, because that's - * how SetConsoleCtrlHandler works. We make it stop worker threads, send - * cancels on all active connections, and then return FALSE, which will allow - * the process to die. For safety's sake, we use a critical section to - * protect the PGcancel structures against being changed while the signal - * thread runs. + * how SetConsoleCtrlHandler works. We make it forcibly terminate the worker + * threads (so they can't report the query cancellations as errors), send + * cancels on all active connections, and then call _exit() to terminate the + * process. Access to the shared PGcancelConn structures is serialized against + * the main thread by the cancel thread lock held around the callback. */ -#ifndef WIN32 - /* - * Signal handler (Unix only) + * Cancel all active queries, print a termination message, and exit. + * + * Invoked from the cancel thread (Unix) or the Windows console handler thread. + * It never returns: after sending the cancels it calls _exit() so that the + * process terminates on cancel. We use _exit() rather than exit() because the + * latter would invoke atexit handlers that can fail if we interrupted related + * code. */ -static void -sigTermHandler(SIGNAL_ARGS) +pg_noreturn static void +CancelBackendsAndExit(void) { +#ifdef WIN32 int i; - char errbuf[1]; /* - * Some platforms allow delivery of new signals to interrupt an active - * signal handler. That could muck up our attempt to send PQcancel, so - * disable the signals that set_cancel_handler enabled. - */ - pqsignal(SIGINT, PG_SIG_IGN); - pqsignal(SIGTERM, PG_SIG_IGN); - pqsignal(SIGQUIT, PG_SIG_IGN); - - /* - * If we're in the leader, forward signal to all workers. (It seems best - * to do this before PQcancel; killing the leader transaction will result - * in invalid-snapshot errors from active workers, which maybe we can - * quiet by killing workers first.) Ignore any errors. + * On Windows the workers are threads within this same process. Once we + * cancel their queries below they would receive the cancellation as an + * error and report it, cluttering the user's screen in the brief window + * before the process exits. Forcibly terminate the worker threads first + * so they can't do that. + * + * TerminateThread is unsafe in general (it may leak resources or leave + * user-space locks held by the killed thread), but that's acceptable here + * because we're about to end the whole process anyway. */ if (signal_info.pstate != NULL) { for (i = 0; i < signal_info.pstate->numWorkers; i++) { - pid_t pid = signal_info.pstate->parallelSlot[i].pid; + HANDLE hThread = (HANDLE) signal_info.pstate->parallelSlot[i].hThread; - if (pid != 0) - kill(pid, SIGTERM); + if (hThread != INVALID_HANDLE_VALUE) + TerminateThread(hThread, 0); } } +#endif /* - * Send QueryCancel if we have a connection to send to. Ignore errors, - * there's not much we can do about them anyway. + * Stop workers first to avoid invalid-snapshot errors if the leader + * cancels before workers. */ - if (signal_info.myAH != NULL && signal_info.myAH->connCancel != NULL) - (void) PQcancel(signal_info.myAH->connCancel, errbuf, sizeof(errbuf)); + StopWorkers(); + + if (signal_info.myAH != NULL && signal_info.myAH->cancelConn != NULL) + (void) PQcancelBlocking(signal_info.myAH->cancelConn); /* - * Report we're quitting, using nothing more complicated than write(2). - * When in parallel operation, only the leader process should do this. + * Print termination message. In parallel operation, only the leader + * should print this. On Windows, workers are threads in the same process + * and the console handler only runs in the leader context, so we can + * always print it. */ +#ifndef WIN32 if (!signal_info.am_worker) +#endif { if (progname) { @@ -596,111 +578,42 @@ sigTermHandler(SIGNAL_ARGS) write_stderr("terminated by user\n"); } - /* - * And die, using _exit() not exit() because the latter will invoke atexit - * handlers that can fail if we interrupted related code. - */ _exit(1); } /* - * Enable cancel interrupt handler, if not already done. - */ -static void -set_cancel_handler(void) -{ - /* - * When forking, signal_info.handler_set will propagate into the new - * process, but that's fine because the signal handler state does too. - */ - if (!signal_info.handler_set) - { - signal_info.handler_set = true; - - pqsignal(SIGINT, sigTermHandler); - pqsignal(SIGTERM, sigTermHandler); - pqsignal(SIGQUIT, sigTermHandler); - } -} - -#else /* WIN32 */ - -/* - * Console interrupt handler --- runs in a newly-started thread. + * Stop all worker processes/threads. * - * After stopping other threads and sending cancel requests on all open - * connections, we return FALSE which will allow the default ExitProcess() - * action to be taken. + * On Unix, send SIGTERM to each worker process; their signal handlers will + * send cancel requests to their backends. + * + * On Windows, workers are threads in the same process, so we send cancel + * requests directly to their backends. + * + * Caller must hold the cancel thread lock (via LockCancelThread). */ -static BOOL WINAPI -consoleHandler(DWORD dwCtrlType) +static void +StopWorkers(void) { int i; - char errbuf[1]; - if (dwCtrlType == CTRL_C_EVENT || - dwCtrlType == CTRL_BREAK_EVENT) - { - /* Critical section prevents changing data we look at here */ - EnterCriticalSection(&signal_info_lock); - - /* - * If in parallel mode, stop worker threads and send QueryCancel to - * their connected backends. The main point of stopping the worker - * threads is to keep them from reporting the query cancels as errors, - * which would clutter the user's screen. We needn't stop the leader - * thread since it won't be doing much anyway. Do this before - * canceling the main transaction, else we might get invalid-snapshot - * errors reported before we can stop the workers. Ignore errors, - * there's not much we can do about them anyway. - */ - if (signal_info.pstate != NULL) - { - for (i = 0; i < signal_info.pstate->numWorkers; i++) - { - ParallelSlot *slot = &(signal_info.pstate->parallelSlot[i]); - ArchiveHandle *AH = slot->AH; - HANDLE hThread = (HANDLE) slot->hThread; - - /* - * Using TerminateThread here may leave some resources leaked, - * but it doesn't matter since we're about to end the whole - * process. - */ - if (hThread != INVALID_HANDLE_VALUE) - TerminateThread(hThread, 0); - - if (AH != NULL && AH->connCancel != NULL) - (void) PQcancel(AH->connCancel, errbuf, sizeof(errbuf)); - } - } + if (signal_info.pstate == NULL) + return; - /* - * Send QueryCancel to leader connection, if enabled. Ignore errors, - * there's not much we can do about them anyway. - */ - if (signal_info.myAH != NULL && signal_info.myAH->connCancel != NULL) - (void) PQcancel(signal_info.myAH->connCancel, - errbuf, sizeof(errbuf)); + for (i = 0; i < signal_info.pstate->numWorkers; i++) + { +#ifndef WIN32 + pid_t pid = signal_info.pstate->parallelSlot[i].pid; - LeaveCriticalSection(&signal_info_lock); + if (pid != 0) + kill(pid, SIGTERM); +#else + ArchiveHandle *AH = signal_info.pstate->parallelSlot[i].AH; - /* - * Report we're quitting, using nothing more complicated than - * write(2). (We might be able to get away with using pg_log_*() - * here, but since we terminated other threads uncleanly above, it - * seems better to assume as little as possible.) - */ - if (progname) - { - write_stderr(progname); - write_stderr(": "); - } - write_stderr("terminated by user\n"); + if (AH != NULL && AH->cancelConn != NULL) + (void) PQcancelBlocking(AH->cancelConn); +#endif } - - /* Always return FALSE to allow signal handling to continue */ - return FALSE; } /* @@ -709,58 +622,55 @@ consoleHandler(DWORD dwCtrlType) static void set_cancel_handler(void) { - if (!signal_info.handler_set) - { - signal_info.handler_set = true; + if (signal_info.handler_set) + return; - InitializeCriticalSection(&signal_info_lock); + signal_info.handler_set = true; - SetConsoleCtrlHandler(consoleHandler, TRUE); - } -} + setup_cancel_handler(NULL, CancelBackendsAndExit); -#endif /* WIN32 */ +#ifndef WIN32 + pqsignal(SIGTERM, CancelSignalHandler); + pqsignal(SIGQUIT, CancelSignalHandler); +#endif +} /* * set_archive_cancel_info * - * Fill AH->connCancel with cancellation info for the specified database + * Fill AH->cancelConn with cancellation info for the specified database * connection; or clear it if conn is NULL. */ void set_archive_cancel_info(ArchiveHandle *AH, PGconn *conn) { - PGcancel *oldConnCancel; + PGcancelConn *oldCancelConn; /* - * Activate the interrupt handler if we didn't yet in this process. On - * Windows, this also initializes signal_info_lock; therefore it's + * Activate the interrupt handler if we didn't yet in this process. It's * important that this happen at least once before we fork off any * threads. */ set_cancel_handler(); /* - * On Unix, we assume that storing a pointer value is atomic with respect - * to any possible signal interrupt. On Windows, use a critical section. + * Use mutex to prevent the cancel handler from using the pointer while + * we're changing it. */ - -#ifdef WIN32 - EnterCriticalSection(&signal_info_lock); -#endif + LockCancelThread(); /* Free the old one if we have one */ - oldConnCancel = AH->connCancel; + oldCancelConn = AH->cancelConn; /* be sure interrupt handler doesn't use pointer while freeing */ - AH->connCancel = NULL; + AH->cancelConn = NULL; - if (oldConnCancel != NULL) - PQfreeCancel(oldConnCancel); + if (oldCancelConn != NULL) + PQcancelFinish(oldCancelConn); /* Set the new one if specified */ if (conn) - AH->connCancel = PQgetCancel(conn); + AH->cancelConn = PQcancelCreate(conn); /* * On Unix, there's only ever one active ArchiveHandle per process, so we @@ -776,49 +686,35 @@ set_archive_cancel_info(ArchiveHandle *AH, PGconn *conn) signal_info.myAH = AH; #endif -#ifdef WIN32 - LeaveCriticalSection(&signal_info_lock); -#endif + UnlockCancelThread(); } /* * set_cancel_pstate * * Set signal_info.pstate to point to the specified ParallelState, if any. - * We need this mainly to have an interlock against Windows signal thread. + * We need this mainly to have an interlock against the cancel handler thread. */ static void set_cancel_pstate(ParallelState *pstate) { -#ifdef WIN32 - EnterCriticalSection(&signal_info_lock); -#endif - + LockCancelThread(); signal_info.pstate = pstate; - -#ifdef WIN32 - LeaveCriticalSection(&signal_info_lock); -#endif + UnlockCancelThread(); } /* * set_cancel_slot_archive * * Set ParallelSlot's AH field to point to the specified archive, if any. - * We need this mainly to have an interlock against Windows signal thread. + * We need this mainly to have an interlock against the cancel handler thread. */ static void set_cancel_slot_archive(ParallelSlot *slot, ArchiveHandle *AH) { -#ifdef WIN32 - EnterCriticalSection(&signal_info_lock); -#endif - + LockCancelThread(); slot->AH = AH; - -#ifdef WIN32 - LeaveCriticalSection(&signal_info_lock); -#endif + UnlockCancelThread(); } @@ -933,7 +829,7 @@ ParallelBackupStart(ArchiveHandle *AH) /* * Temporarily disable query cancellation on the leader connection. This - * ensures that child processes won't inherit valid AH->connCancel + * ensures that child processes won't inherit valid AH->cancelConn * settings and thus won't try to issue cancels against the leader's * connection. No harm is done if we fail while it's disabled, because * the leader connection is idle at this point anyway. @@ -951,6 +847,7 @@ ParallelBackupStart(ArchiveHandle *AH) uintptr_t handle; #else pid_t pid; + sigset_t cancel_set; #endif ParallelSlot *slot = &(pstate->parallelSlot[i]); int pipeMW[2], @@ -979,6 +876,18 @@ ParallelBackupStart(ArchiveHandle *AH) slot->hThread = handle; slot->workerStatus = WRKR_IDLE; #else /* !WIN32 */ + + /* + * Block signals before fork so that no signal can arrive in the child + * before ResetCancelAfterFork() has cleaned up the inherited cancel + * state (pipe fds, signal handlers, mutex). + */ + sigemptyset(&cancel_set); + sigaddset(&cancel_set, SIGINT); + sigaddset(&cancel_set, SIGTERM); + sigaddset(&cancel_set, SIGQUIT); + sigprocmask(SIG_BLOCK, &cancel_set, NULL); + pid = fork(); if (pid == 0) { @@ -991,6 +900,12 @@ ParallelBackupStart(ArchiveHandle *AH) /* instruct signal handler that we're in a worker now */ signal_info.am_worker = true; + signal_info.handler_set = false; + ResetCancelAfterFork(); + pqsignal(SIGTERM, PG_SIG_DFL); + pqsignal(SIGQUIT, PG_SIG_DFL); + sigprocmask(SIG_UNBLOCK, &cancel_set, NULL); + /* close read end of Worker -> Leader */ closesocket(pipeWM[PIPE_READ]); /* close write end of Leader -> Worker */ @@ -1019,6 +934,8 @@ ParallelBackupStart(ArchiveHandle *AH) } /* In Leader after successful fork */ + sigprocmask(SIG_UNBLOCK, &cancel_set, NULL); + slot->pid = pid; slot->workerStatus = WRKR_IDLE; @@ -1407,8 +1324,12 @@ ListenToWorkers(ArchiveHandle *AH, ParallelState *pstate, bool do_wait) if (!msg) { - /* If do_wait is true, we must have detected EOF on some socket */ - if (do_wait) + /* + * If do_wait is true, we must have detected EOF on some socket. If + * it's due to a cancel request, that's expected, otherwise it's a + * problem. + */ + if (do_wait && !CancelRequested) pg_fatal("a worker process died unexpectedly"); return false; } diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 46f4c518347..18ec9320166 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -5196,7 +5196,7 @@ CloneArchive(ArchiveHandle *AH) /* The clone will have its own connection, so disregard connection state */ clone->connection = NULL; - clone->connCancel = NULL; + clone->cancelConn = NULL; clone->currUser = NULL; clone->currSchema = NULL; clone->currTableAm = NULL; diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h index c1528d78853..28febac0c43 100644 --- a/src/bin/pg_dump/pg_backup_archiver.h +++ b/src/bin/pg_dump/pg_backup_archiver.h @@ -288,8 +288,12 @@ struct _archiveHandle char *savedPassword; /* password for ropt->username, if known */ char *use_role; PGconn *connection; - /* If connCancel isn't NULL, SIGINT handler will send a cancel */ - PGcancel *volatile connCancel; + + /* + * If cancelConn isn't NULL, SIGINT handler will trigger the cancel thread + * to send a cancel. + */ + PGcancelConn *cancelConn; int connectToDB; /* Flag to indicate if direct DB connection is * required */ diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 5c349279beb..0cc29a8aa70 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -84,7 +84,7 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname) /* * Note: we want to establish the new connection, and in particular update - * ArchiveHandle's connCancel, before closing old connection. Otherwise + * ArchiveHandle's cancelConn, before closing old connection. Otherwise * an ill-timed SIGINT could try to access a dead connection. */ AH->connection = NULL; /* dodge error check in ConnectDatabaseAhx */ @@ -164,12 +164,11 @@ void DisconnectDatabase(Archive *AHX) { ArchiveHandle *AH = (ArchiveHandle *) AHX; - char errbuf[1]; if (!AH->connection) return; - if (AH->connCancel) + if (AH->cancelConn) { /* * If we have an active query, send a cancel before closing, ignoring @@ -177,7 +176,7 @@ DisconnectDatabase(Archive *AHX) * helpful during pg_fatal(). */ if (PQtransactionStatus(AH->connection) == PQTRANS_ACTIVE) - (void) PQcancel(AH->connCancel, errbuf, sizeof(errbuf)); + (void) PQcancelBlocking(AH->cancelConn); /* * Prevent signal handler from sending a cancel after this. diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 0b2bb9340b5..85116e674f4 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -5336,7 +5336,7 @@ runInitSteps(const char *initialize_steps) if ((con = doConnect()) == NULL) pg_fatal("could not create connection for initialization"); - setup_cancel_handler(NULL); + setup_cancel_handler(NULL, NULL); SetCancelConn(con); for (step = initialize_steps; *step != '\0'; step++) diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 660f14559f5..fc3e020d6e9 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -305,23 +305,27 @@ volatile sig_atomic_t sigint_interrupt_enabled = false; sigjmp_buf sigint_interrupt_jmp; +#ifndef WIN32 static void psql_cancel_callback(void) { -#ifndef WIN32 /* if we are waiting for input, longjmp out of it */ if (sigint_interrupt_enabled) { sigint_interrupt_enabled = false; siglongjmp(sigint_interrupt_jmp, 1); } -#endif } +#endif void psql_setup_cancel_handler(void) { - setup_cancel_handler(psql_cancel_callback); +#ifndef WIN32 + setup_cancel_handler(psql_cancel_callback, NULL); +#else + setup_cancel_handler(NULL, NULL); +#endif } diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c index 53bbb42c883..f282a88c76d 100644 --- a/src/bin/scripts/clusterdb.c +++ b/src/bin/scripts/clusterdb.c @@ -140,7 +140,7 @@ main(int argc, char *argv[]) cparams.prompt_password = prompt_password; cparams.override_dbname = NULL; - setup_cancel_handler(NULL); + setup_cancel_handler(NULL, NULL); if (alldb) { diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index d7fb16d3c85..75613b995ee 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -211,7 +211,7 @@ main(int argc, char *argv[]) cparams.prompt_password = prompt_password; cparams.override_dbname = NULL; - setup_cancel_handler(NULL); + setup_cancel_handler(NULL, NULL); if (concurrentCons > 1 && syscatalog) pg_fatal("cannot use multiple jobs to reindex system catalogs"); diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index 67a7665c5d7..1b08423f422 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -58,7 +58,7 @@ vacuuming_main(ConnParams *cparams, const char *dbname, unsigned int tbl_count, int concurrentCons, const char *progname) { - setup_cancel_handler(NULL); + setup_cancel_handler(NULL, NULL); /* Avoid opening extra connections. */ if (tbl_count > 0 && (concurrentCons > tbl_count)) diff --git a/src/fe_utils/Makefile b/src/fe_utils/Makefile index cbfbf93ac69..809ab21cc0c 100644 --- a/src/fe_utils/Makefile +++ b/src/fe_utils/Makefile @@ -17,7 +17,7 @@ subdir = src/fe_utils top_builddir = ../.. include $(top_builddir)/src/Makefile.global -override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) $(CPPFLAGS) +override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) -I$(top_srcdir)/src/port $(CPPFLAGS) OBJS = \ archive.o \ diff --git a/src/fe_utils/cancel.c b/src/fe_utils/cancel.c index e6b75439f56..f40c4242145 100644 --- a/src/fe_utils/cancel.c +++ b/src/fe_utils/cancel.c @@ -2,9 +2,57 @@ * * Query cancellation support for frontend code * - * Assorted utility functions to control query cancellation with signal - * handler for SIGINT. + * This module provides SIGINT/Ctrl-C handling for frontend tools that need + * to cancel queries or interrupt other operations. It combines four completely + * independent mechanisms, any combination of which can be used by a caller: * + * 1. Server cancel query request -- Often what applications need. When a query + * is running, and the main thread is waiting for the result of that query + * in a blocking manner, we want SIGINT/Ctrl-C to cancel that query. This + * can be done by having the application call SetCancelConn() to register + * the connection that is (or will be) running the query, prior to waiting + * for the result. When SIGINT/Ctrl-C is received a cancel request for this + * connection will then be sent to the server from a separate thread. That + * in turn will then (assuming a co-operating server) cause the server to + * cancel the query and send an error to the waiting client on the main + * thread. The cancel connection is a process-wide global, so only one + * connection can be the cancel target at a time. ResetCancelConn() can be + * used to unregister the connection again, preventing sending a cancel + * request if SIGINT/Ctrl-C is received after blocking wait has already + * completed. + * + * 2. CancelRequested flag -- A more involved but also much more flexible way + * of cancelling an operation. A volatile sig_atomic_t CancelRequested flag + * is set to true whenever SIGINT is received. This means that the + * application code can fully control what it does with this flag. The + * primary usecase for this is when the application code is not blocked + * (indefinitely), but needs to take an action when Ctrl-C is pressed, such + * as break out of a long running loop. + * + * 3. Thread handler callback -- An optional function pointer registered via + * setup_cancel_handler(). If set, this function is called from a separate + * thread when a cancel signal is received. If multiple signals are received + * in quick succession, the callback may be called only once. On Windows, + * this is called from the console handler thread. On Unix, this is called + * from the cancel thread that is woken by the signal handler. To ensure + * safe access to shared data, the cancel thread holds the cancel thread + * lock for the duration of the callback, so any other threads that need + * to access the same data should also acquire that lock using + * LockCancelThread()/UnlockCancelThread(). + * + * 4. Signal handler callback -- The most complex way of canceling an + * operation, which is not supported on Windows. An optional signal_callback + * function pointer can be registered via setup_cancel_handler(). If set, + * it is called directly from the signal handler, so it must be + * async-signal-safe. Writing async-signal-safe code is not easy, so this is + * only recommended as a last resort. psql uses this to longjmp back to the + * main loop when no query is active. On Windows, this function is never + * called, since the console handler runs in a separate thread, not a signal + * handler. + * NOTE: The signal handler callback is called AFTER setting CancelRequested + * but BEFORE notifying the cancel thread to send a cancel request to the + * server (if armed by SetCancelConn). This means that if the callback exits + * or longjmps no cancel request will be sent to the server. * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -16,9 +64,21 @@ #include "postgres_fe.h" +#include <signal.h> #include <unistd.h> +#ifndef WIN32 +#include <fcntl.h> +#endif + +#ifdef WIN32 +#include "pthread-win32.h" +#else +#include <pthread.h> +#endif + #include "common/connect.h" +#include "common/logging.h" #include "fe_utils/cancel.h" #include "fe_utils/string_utils.h" @@ -36,11 +96,19 @@ (void) rc_; \ } while (0) + /* - * Contains all the information needed to cancel a query issued from - * a database connection to the backend. + * Cancel connection that should be used to send cancel requests. */ -static PGcancel *volatile cancelConn = NULL; +static PGcancelConn *cancelConn = NULL; + +/* + * Mutex held by the cancel thread for the duration of the cancel callback. + * SetCancelConn()/ResetCancelConn() on the main thread take this lock too, + * so they will wait for any in-flight cancel to finish before replacing or + * freeing cancelConn. + */ +static pthread_mutex_t cancel_thread_lock = PTHREAD_MUTEX_INITIALIZER; /* * Predetermined localized error strings --- needed to avoid trying @@ -58,168 +126,180 @@ static const char *cancel_not_sent_msg = NULL; */ volatile sig_atomic_t CancelRequested = false; -#ifdef WIN32 -static CRITICAL_SECTION cancelConnLock; -#endif +/* + * Signal handler callback, called directly from signal handler context. + * Must be async-signal-safe. + */ +static void (*signal_callback_fn) (void) = NULL; + +/* + * Cancel thread callback, called from the cancel thread (Unix) or console + * handler (Windows) when a cancel signal is received. + */ +static void (*thread_callback_fn) (void) = NULL; +#ifndef WIN32 /* - * Additional callback for cancellations. + * On Unix, the SIGINT signal handler cannot call PQcancelBlocking() directly + * because it is not async-signal-safe. Instead, we use a pipe to wake a + * dedicated cancel thread: the signal handler writes a byte to the pipe, and + * the cancel thread's blocking read() returns, triggering the actual cancel + * request. */ -static void (*cancel_callback) (void) = NULL; +static int cancel_pipe[2] = {-1, -1}; +#endif /* - * SetCancelConn + * Send a cancel request to the connection, if one is set. * - * Set cancelConn to point to the current database connection. + * Called from the cancel thread (Unix) or the console handler thread + * (Windows), never from the signal handler itself. The caller is + * responsible for holding cancel_thread_lock. */ -void -SetCancelConn(PGconn *conn) +static void +SendCancelRequest(void) { - PGcancel *oldCancelConn; - -#ifdef WIN32 - EnterCriticalSection(&cancelConnLock); -#endif - - /* Free the old one if we have one */ - oldCancelConn = cancelConn; + PGcancelConn *cc; - /* be sure handle_sigint doesn't use pointer while freeing */ - cancelConn = NULL; + cc = cancelConn; + if (cc == NULL) + return; - if (oldCancelConn != NULL) - PQfreeCancel(oldCancelConn); + write_stderr(cancel_sent_msg); - cancelConn = PQgetCancel(conn); + if (!PQcancelBlocking(cc)) + { + char *errmsg = PQcancelErrorMessage(cc); -#ifdef WIN32 - LeaveCriticalSection(&cancelConnLock); -#endif + write_stderr(cancel_not_sent_msg); + if (errmsg) + write_stderr(errmsg); + } + /* Reset for possible reuse */ + PQcancelReset(cc); } + /* - * ResetCancelConn + * Helper to replace cancelConn with a new value. * - * Free the current cancel connection, if any, and set to NULL. + * Takes cancel_thread_lock, which also waits for any in-flight cancel + * callback to finish, since the cancel thread holds the same lock. */ -void -ResetCancelConn(void) +static void +SetCancelConnInternal(PGcancelConn *newCancelConn) { - PGcancel *oldCancelConn; - -#ifdef WIN32 - EnterCriticalSection(&cancelConnLock); -#endif + PGcancelConn *oldCancelConn; + LockCancelThread(); oldCancelConn = cancelConn; - - /* be sure handle_sigint doesn't use pointer while freeing */ - cancelConn = NULL; + cancelConn = newCancelConn; + UnlockCancelThread(); if (oldCancelConn != NULL) - PQfreeCancel(oldCancelConn); - -#ifdef WIN32 - LeaveCriticalSection(&cancelConnLock); -#endif + PQcancelFinish(oldCancelConn); } - /* - * Code to support query cancellation - * - * Note that sending the cancel directly from the signal handler is safe - * because PQcancel() is written to make it so. We use write() to report - * to stderr because it's better to use simple facilities in a signal - * handler. + * SetCancelConn * - * On Windows, the signal canceling happens on a separate thread, because - * that's how SetConsoleCtrlHandler works. The PQcancel function is safe - * for this (unlike PQrequestCancel). However, a CRITICAL_SECTION is required - * to protect the PGcancel structure against being changed while the signal - * thread is using it. + * Set cancelConn to point to a cancel connection for the given database + * connection. This creates a new PGcancelConn that can be used to send + * cancel requests. */ - -#ifndef WIN32 +void +SetCancelConn(PGconn *conn) +{ + SetCancelConnInternal(PQcancelCreate(conn)); +} /* - * handle_sigint + * ResetCancelConn * - * Handle interrupt signals by canceling the current command, if cancelConn - * is set. + * Clear cancelConn, preventing any pending cancel from being sent. + * Waits for any in-flight cancel request to complete first. */ -static void -handle_sigint(SIGNAL_ARGS) +void +ResetCancelConn(void) { - char errbuf[256]; + SetCancelConnInternal(NULL); +} - CancelRequested = true; - if (cancel_callback != NULL) - cancel_callback(); +/* + * LockCancelThread / UnlockCancelThread + * + * Acquire or release cancel_thread_lock. External callers (e.g. pg_dump) + * use these to protect shared data that the cancel-thread callback also + * accesses, without exposing the mutex directly. + */ +void +LockCancelThread(void) +{ + pthread_mutex_lock(&cancel_thread_lock); +} - /* Send QueryCancel if we are processing a database query */ - if (cancelConn != NULL) - { - if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) - { - write_stderr(cancel_sent_msg); - } - else - { - write_stderr(cancel_not_sent_msg); - write_stderr(errbuf); - } - } +void +UnlockCancelThread(void) +{ + pthread_mutex_unlock(&cancel_thread_lock); } +#ifndef WIN32 /* - * setup_cancel_handler + * ResetCancelAfterFork + * + * Reset cancel module state after fork(). Threads don't survive fork(), so the + * cancel thread and its pipe are gone. The mutex may have been held by the + * cancel thread at fork time, so we must reinitialize it rather than trying to + * unlock it. cancelConn is NULLed without freeing because the parent process + * owns the underlying object. The SIGINT handler is reset to SIG_DFL so that + * a signal arriving before setup_cancel_handler() is called again doesn't try + * to write to the closed pipe. * - * Register query cancellation callback for SIGINT. + * The child will set up a fresh cancel thread when it later calls + * setup_cancel_handler(). */ void -setup_cancel_handler(void (*query_cancel_callback) (void)) +ResetCancelAfterFork(void) { - cancel_callback = query_cancel_callback; - cancel_sent_msg = _("Cancel request sent\n"); - cancel_not_sent_msg = _("Could not send cancel request: "); + close(cancel_pipe[0]); + close(cancel_pipe[1]); + cancel_pipe[0] = cancel_pipe[1] = -1; - pqsignal(SIGINT, handle_sigint); -} + pthread_mutex_init(&cancel_thread_lock, NULL); -#else /* WIN32 */ + cancelConn = NULL; + CancelRequested = false; + pqsignal(SIGINT, PG_SIG_DFL); +} +#endif + +#ifdef WIN32 +/* + * Console control handler for Windows. + * + * This runs in a separate thread created by the OS, so we can safely call + * the blocking cancel API directly. + */ static BOOL WINAPI consoleHandler(DWORD dwCtrlType) { - char errbuf[256]; - if (dwCtrlType == CTRL_C_EVENT || dwCtrlType == CTRL_BREAK_EVENT) { CancelRequested = true; - if (cancel_callback != NULL) - cancel_callback(); + LockCancelThread(); - /* Send QueryCancel if we are processing a database query */ - EnterCriticalSection(&cancelConnLock); - if (cancelConn != NULL) - { - if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) - { - write_stderr(cancel_sent_msg); - } - else - { - write_stderr(cancel_not_sent_msg); - write_stderr(errbuf); - } - } + SendCancelRequest(); + + if (thread_callback_fn != NULL) + thread_callback_fn(); - LeaveCriticalSection(&cancelConnLock); + UnlockCancelThread(); return TRUE; } @@ -228,16 +308,169 @@ consoleHandler(DWORD dwCtrlType) return FALSE; } +#else /* !WIN32 */ + +/* + * Signal handler that setup_cancel_handler configures for SIGINT. Exposed so + * other signals than SIGINT can use it if desired. + */ void -setup_cancel_handler(void (*callback) (void)) +CancelSignalHandler(SIGNAL_ARGS) { - cancel_callback = callback; - cancel_sent_msg = _("Cancel request sent\n"); - cancel_not_sent_msg = _("Could not send cancel request: "); + int save_errno = errno; - InitializeCriticalSection(&cancelConnLock); + CancelRequested = true; - SetConsoleCtrlHandler(consoleHandler, TRUE); + if (signal_callback_fn != NULL) + signal_callback_fn(); + + /* Wake up the cancel thread */ + if (cancel_pipe[1] >= 0) + { + char c = 1; + int rc = write(cancel_pipe[1], &c, 1); + + (void) rc; + } + + errno = save_errno; +} + +/* + * Thread main function for create_cancel_thread. Waits for the signal + * handler to write a byte to the pipe, then calls the cancel callback. + */ +static void * +cancel_thread_loop(void *arg) +{ + for (;;) + { + char buf[16]; + ssize_t rc; + + rc = read(cancel_pipe[0], buf, sizeof(buf)); + if (rc <= 0) + { + if (errno == EINTR) + continue; + /* Pipe closed or error - exit thread */ + break; + } + + LockCancelThread(); + + SendCancelRequest(); + + if (thread_callback_fn != NULL) + thread_callback_fn(); + + /* + * Drain any pending bytes from the cancel pipe, so that signals + * received while we were already handling a cancel don't cause us to + * wake up again and cancel a subsequent query. + */ + fcntl(cancel_pipe[0], F_SETFL, O_NONBLOCK); + while (read(cancel_pipe[0], buf, sizeof(buf)) > 0) + ; /* loop until pipe is fully drained */ + fcntl(cancel_pipe[0], F_SETFL, 0); + + UnlockCancelThread(); + } + + return NULL; +} + +/* + * create_cancel_thread + * + * Create a dedicated thread and associated pipe for async-signal-safe cancel + * handling. The pipe allows signal handlers (which cannot safely call complex + * functions) to wake up the thread by writing a byte. + * + * The write end of the pipe is set non-blocking so signal handlers never + * block. The thread is created with all signals blocked so that signals are + * always delivered to the main thread. The thread runs until process exit. + * No handle is returned because currently no callers need to join it. + */ +static void +create_cancel_thread(void) +{ + sigset_t save_set; + sigset_t block_set; + pthread_t thread; + int rc; + + if (pipe(cancel_pipe) < 0) + { + pg_log_error("could not create pipe for cancel: %m"); + exit(1); + } + + /* + * Make the write end non-blocking, so that the signal handler won't block + * if the pipe buffer is full (which is very unlikely in practice but + * possible in theory). + */ + fcntl(cancel_pipe[1], F_SETFL, O_NONBLOCK); + + /* + * Block all signals before creating the cancel thread, so that it + * inherits a signal mask with all signals blocked. This ensures signals + * are always delivered to the main thread, which matters because some + * signal_callback functions call siglongjmp() back to a sigsetjmp() on + * the main thread's stack, specifically the psql_cancel_callback + * function. + */ + sigfillset(&block_set); + pthread_sigmask(SIG_BLOCK, &block_set, &save_set); + + rc = pthread_create(&thread, NULL, cancel_thread_loop, NULL); + + pthread_sigmask(SIG_SETMASK, &save_set, NULL); + + if (rc != 0) + { + pg_log_error("could not create cancel thread: %s", strerror(rc)); + exit(1); + } + + pthread_detach(thread); } -#endif /* WIN32 */ +#endif /* !WIN32 */ + + +/* + * setup_cancel_handler + * + * Set up signal handling for SIGINT (Unix) or console events (Windows) to + * perform cancel actions. + * + * signal_callback is invoked directly from the signal handler context on + * every SIGINT (on Unix), so it must be async-signal-safe. Can be NULL. + * On Windows, signal handlers don't exist (the console handler runs in a + * separate thread), so signal_callback must be NULL. + * + * thread_callback is invoked from a dedicated cancel thread (Unix) or the + * console handler thread (Windows) when a signal is received. Can be NULL. + */ +void +setup_cancel_handler(void (*signal_callback) (void), + void (*thread_callback) (void)) +{ +#ifdef WIN32 + Assert(signal_callback == NULL); +#endif + + signal_callback_fn = signal_callback; + thread_callback_fn = thread_callback; + cancel_sent_msg = _("Sending cancel request\n"); + cancel_not_sent_msg = _("Could not send cancel request: "); + +#ifdef WIN32 + SetConsoleCtrlHandler(consoleHandler, TRUE); +#else + create_cancel_thread(); + pqsignal(SIGINT, CancelSignalHandler); +#endif +} diff --git a/src/include/fe_utils/cancel.h b/src/include/fe_utils/cancel.h index e174fb83b92..feb1970d372 100644 --- a/src/include/fe_utils/cancel.h +++ b/src/include/fe_utils/cancel.h @@ -23,10 +23,15 @@ extern PGDLLIMPORT volatile sig_atomic_t CancelRequested; extern void SetCancelConn(PGconn *conn); extern void ResetCancelConn(void); -/* - * A callback can be optionally set up to be called at cancellation - * time. - */ -extern void setup_cancel_handler(void (*query_cancel_callback) (void)); +extern void setup_cancel_handler(void (*signal_callback) (void), + void (*thread_callback) (void)); + +extern void LockCancelThread(void); +extern void UnlockCancelThread(void); + +#ifndef WIN32 +extern void ResetCancelAfterFork(void); +extern void CancelSignalHandler(SIGNAL_ARGS); +#endif #endif /* CANCEL_H */ -- 2.54.0
