tcdi / plrust

A Rust procedural language handler for PostgreSQL
PostgreSQL License
1.1k stars 32 forks source link

seg faults when working with composite types #411

Open darrenmothersele opened 3 months ago

darrenmothersele commented 3 months ago

Hi,

I am getting seg faults from plrust code when working with composite types. I've seen this when working locally (in Docker), using cloudnative-pg, and in RDS on AWS (so I'm pretty sure it's not an environment issue).

I have a few cases where this occurs and I have not been able to track down what is causing it. I think it might be related to PgHeapTuple<AllocatedByRust>. I have seen it when calling the functions directly, but it usually occurs when calling a plrust function from a plpgsql function. Here's a minimal example that triggers it...

DROP TYPE IF EXISTS testevent CASCADE;
create type testevent AS
(
    entity_id text
);

DROP FUNCTION IF EXISTS test_fault_inner;
CREATE OR REPLACE FUNCTION test_fault_inner(entity_id text)
    RETURNS testevent
    LANGUAGE plrust AS
$_$
    let entity_id = entity_id.unwrap();

    let mut e = PgHeapTuple::new_composite_type("testevent")?;
    Spi::connect(|mut client| {
        e.set_by_name("entity_id", entity_id)?;
        Ok(Some(e))
    })
$_$;

DROP FUNCTION IF EXISTS test_fault;
CREATE OR REPLACE FUNCTION test_fault(id text)
    RETURNS void
    LANGUAGE plpgsql AS
$_$
DECLARE
    temp testevent;
BEGIN
    temp = test_fault_inner(id);
    RAISE LOG '%', temp.entity_id;
end
$_$;

SELECT test_fault(gen_random_uuid()::text);
SELECT test_fault(gen_random_uuid()::text);

Hopefully I'm holding it wrong.

workingjubilee commented 3 months ago

@darrenmothersele It should not be possible for you to "hold it wrong" and get a segfault. Doing so means a bug in our code or a bug in Postgres.

I am unfortunately aware of what bug you probably have hit.

Do you have any reproducers that do not use SPI in the Rust function?

There is a known soundness problem with SPI in pgrx, which PL/Rust depends on. It has to do with entering SPI, which creates a new memory context, and everything created in that memory context needs to be appropriately bounded by that context. I have been hammering away at new API which can represent memory contexts and their lifetimes but there are a few more steps until I can land the fixes in PL/Rust.

That said if you have a reproducer that does not require such, then I am very interested, as that's a new bug.

darrenmothersele commented 3 weeks ago

Hi, just wanted to confirm that I think our issues were related to the SPI soundness problem.

Thanks

workingjubilee commented 2 weeks ago

Cool! Good to know. pgrx 0.12 doesn't address that, unfortunately, but did lay all the preliminary groundwork for such.