From 81dcc93bfc1345c64a2004dd7498a2d13b9aca40 Mon Sep 17 00:00:00 2001 From: Eric Ridge Date: Tue, 23 May 2023 12:47:49 -0400 Subject: [PATCH] fix issue #1141: immediately report FATAL and PANIC (#1142) errors reported at the FATAL or PANIC levels should immediately be reported to Postgres since they ultimately abort the transaction. ERRORs continue to be converted into panics, and everything else is also immediately reported to Postgres but since they only emit messages, there's no change to flow control. As a drive-by, I also added a few functions to the "bad_ideas" example to interactively test this change. It's not really feasible for the test suite to somehow test that a test causes the backend, or the entire cluster, to crash, which is what PANIC and FATAL do. --- pgrx-examples/bad_ideas/src/lib.rs | 37 +++++++++++++++++++++++++++++ pgrx-pg-sys/src/submodules/panic.rs | 20 +++++++++------- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/pgrx-examples/bad_ideas/src/lib.rs b/pgrx-examples/bad_ideas/src/lib.rs index 369c7ce5e..78f64742d 100644 --- a/pgrx-examples/bad_ideas/src/lib.rs +++ b/pgrx-examples/bad_ideas/src/lib.rs @@ -10,10 +10,47 @@ use pgrx::prelude::*; use pgrx::{check_for_interrupts, info, register_xact_callback, PgRelation, PgXactCallbackEvent}; use std::fs::File; use std::io::Write; +use std::panic::catch_unwind; use std::process::Command; pgrx::pg_module_magic!(); +#[pg_extern] +fn panic(s: &str) -> bool { + catch_unwind(|| { + PANIC!("{}", s); + }) + .ok(); + true +} + +#[pg_extern] +fn fatal(s: &str) -> bool { + catch_unwind(|| { + FATAL!("{}", s); + }) + .ok(); + true +} + +#[pg_extern] +fn error(s: &str) -> bool { + catch_unwind(|| { + error!("{}", s); + }) + .ok(); + true +} + +#[pg_extern] +fn warning(s: &str) -> bool { + catch_unwind(|| { + warning!("{}", s); + }) + .ok(); + true +} + #[pg_extern] fn exec<'a>( command: &'a str, diff --git a/pgrx-pg-sys/src/submodules/panic.rs b/pgrx-pg-sys/src/submodules/panic.rs index cd5738fa4..d4984a3ca 100644 --- a/pgrx-pg-sys/src/submodules/panic.rs +++ b/pgrx-pg-sys/src/submodules/panic.rs @@ -152,14 +152,18 @@ pub struct ErrorReportWithLevel { impl ErrorReportWithLevel { fn report(self) { - // ONLY if the log level is >=ERROR, we convert ourselves into a Rust panic and ask - // rust to raise us as a `panic!()` - // - // Lesser levels (INFO, WARNING, LOG, etc) will just emit a message which isn't a panic condition - if crate::ERROR <= self.level as _ { - panic_any(self) - } else { - do_ereport(self) + match self.level { + // ERRORs get converted into panics so they can perform proper stack unwinding + PgLogLevel::ERROR => panic_any(self), + + // FATAL and PANIC are reported directly to Postgres -- they abort the process + PgLogLevel::FATAL | PgLogLevel::PANIC => { + do_ereport(self); + unreachable!() + } + + // Everything else (INFO, WARN, LOG, DEBUG, etc) are reported to Postgres too but they only emit messages + _ => do_ereport(self), } }