fix: crash from drop impls that panic via leak_and_drop_on_delete() (…

…#1929)

If `PgMemoryContexts::leak_and_drop_on_delete()` leaks a type that is
`impl Drop`, and that implementation happens to panic, then Postgres
will segfault because the callback function was not properly guarded.

As a drive-by, slightly cleanup error reporting in the test framework.

pgrx-tests/src/framework.rs

@@ -646,7 +646,11 @@ fn monitor_pg(mut command: Command, cmd_string: String, loglines: LogLines) -> S
 
             if line.contains("database system is ready to accept connections") {
                 // Postgres says it's ready to go
-                sender.send(session_id.clone()).unwrap();
+                if let Err(_) = sender.send(session_id.clone()) {
+                    // The channel is closed.  This is really early in the startup process
+                    // and likely indicates that a test crashed Postgres
+                    panic!("{}: `monitor_pg()`:  failed to send back session_id `{session_id}`.  Did Postgres crash?", "ERROR".red().bold());
+                }
                 is_started_yet = true;
             }
 

pgrx-tests/src/tests/memcxt_tests.rs

@@ -50,6 +50,50 @@ mod tests {
         assert!(did_drop.load(Ordering::SeqCst))
     }
 
+    #[pg_test]
+    fn test_leak_and_drop_with_panic() {
+        let result = std::panic::catch_unwind(|| unsafe {
+            struct Thing;
+            impl Drop for Thing {
+                fn drop(&mut self) {
+                    panic!("please don't crash")
+                }
+            }
+
+            PgMemoryContexts::Transient {
+                parent: PgMemoryContexts::CurrentMemoryContext.value(),
+                name: "test",
+                min_context_size: 4096,
+                initial_block_size: 4096,
+                max_block_size: 4096,
+            }
+            .switch_to(|context| {
+                context.leak_and_drop_on_delete(Thing);
+            });
+        });
+
+        assert!(result.is_err());
+        let err = result.unwrap_err();
+        let caught = err.downcast_ref::<pg_sys::panic::CaughtError>();
+
+        match caught {
+            // would indicate some kind of terribleness with pgrx panic handling
+            None => panic!("caught a panic that isn't a `pg_sys::panic::CaughtError`"),
+
+            // this is the type of error we expect.  It comes to us here as the PostgresError
+            // variant because, while it was a Rust panic, it was from a function with #[pg_guard]
+            // directly called from Postgres
+            Some(&pg_sys::panic::CaughtError::PostgresError(ref report))
+                if report.message() == "please don't crash" =>
+            {
+                // ok!
+            }
+
+            // got some other kind of CaughtError variant we shouldn't have
+            _ => panic!("did not catch the correct error/panic type: {caught:?}"),
+        }
+    }
+
     #[pg_test]
     fn parent() {
         unsafe {

pgrx/src/memcxt.rs

@@ -555,6 +555,8 @@ impl PgMemoryContexts {
     /// this MemoryContext, the original instance of `T` will be resurrected and its `impl Drop`
     /// will be called.
     pub fn leak_and_drop_on_delete<T>(&mut self, v: T) -> *mut T {
+        use crate as pgrx;
+        #[pgrx::pg_guard]
         unsafe extern "C" fn drop_on_delete<T>(ptr: void_mut_ptr) {
             let boxed = Box::from_raw(ptr as *mut T);
             drop(boxed);