From 51382cf4407f9b924b649c97c8e59b3024c05925 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio 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(" \n", fout); } @@ -2046,7 +2043,7 @@ print_html_text(const printTableContent *cont, FILE *fout) fputs("\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("\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