Skip to content

Add MemCx<'current, T> arguments and PBox<'current, T> returns#2210

Merged
workingjubilee merged 23 commits into
pgcentralfoundation:developfrom
workingjubilee:memcx-virtual-arg
Nov 22, 2025
Merged

Add MemCx<'current, T> arguments and PBox<'current, T> returns#2210
workingjubilee merged 23 commits into
pgcentralfoundation:developfrom
workingjubilee:memcx-virtual-arg

Conversation

@workingjubilee

@workingjubilee workingjubilee commented Nov 17, 2025

Copy link
Copy Markdown
Member

tl;dr: caller_picks_lifetime + macro_expansion == unsoundness,
so prevent that by adding API for new, lifetime-bound allocations.

For functions exposed to PostgreSQL via pg_extern, signatures often resemble these:

#[pg_extern]
fn do_something(
    floats: Array<'a, f32>,
    hms: Time,
    day: TimestampTz
) -> Vec<TimestampTz> {
-- pgrx will generate something like this
CREATE FUNCTION do_something(
    floats real[],
    hms time,
    day timestamptz
) RETURNS timestamptz[]

In SQL, we accept an array and return an array of a different type, but in Rust, we return a Vec. This is because we lack API to allocate and return a fresh Array<'a, T>s. Behind the scenes, we do allocate an ArrayType at the translation boundary, unseen to a Rust programmer. That, plus the Vec, means two allocations just to mutate an array, even for a fixed-size type!

Unfortunately, pgrx-offered type translations like Array<'a, T>, cannot have a lifetime and expose a new safe function to make it from lifetime-free values! Such API must have the unsafe requirement of access-exclusivity for its lifetime, described in functions like slice::from_raw_parts_mut1, because the lifetime becomes caller-selected. This makes it easy to use a 'static lifetime with pg_extern and expand unsafe code that trusts that lifetime.

A 'static lifetime is always wrong for pallocated types, even with TopMemoryContext, since it dies whenever Postgres resets the context and Postgres does have conditions under which it performs such a global reset. The result is various extremely subtle soundness problems are possible, yet the root cause lies in code that most never read. We can solve this by denying the possibility of a caller-selected lifetime. This will require removing some types, ultimately, but first we need an API to migrate to.

This new API is MemCx<'mcx, T> and PBox<'mcx, T>. Importantly, there is no safe route to creating MemCx<'mcx, T> with a caller-selected lifetime. One can only reach MemCx<'mcx, T> safely as an argument: either pass a closure to memcx::current_context or call a new pg_extern function via SQL. Either way gives you &MemCx<'current>. These let you create a lifetime-bound allocation that can live long enough to return it.

We can already return fixed-size pass-by-reference types, so right now PBox is only an optimization. It allows not putting a type on the stack and then immediately moving it directly into a heap allocation. More importantly, it enables writing tests, so it is important ground work for returning de facto unsized values, as they can hide behind PBox which is always Sized.

Footnotes

  1. https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html

@workingjubilee workingjubilee changed the title Memcx virtual arg Allow &MemCx<'_> as an argument Nov 17, 2025
@workingjubilee

Copy link
Copy Markdown
Member Author

moving PBox work into this PR since it should probably work for things that aren't just FlatArray.

@workingjubilee workingjubilee changed the title Allow &MemCx<'_> as an argument Add MemCx<'current, T> arguments and PBox<'current, T> returns Nov 18, 2025
@workingjubilee

workingjubilee commented Nov 18, 2025

Copy link
Copy Markdown
Member Author
        fn_args: <[_]>::into_vec(::alloc::boxed::box_new([
            ::pgrx::pgrx_sql_entity_graph::PgExternArgumentEntity {
                pattern: "memcx",
                used_ty: ::pgrx::pgrx_sql_entity_graph::UsedTypeEntity {
                    ty_source: "& '_ MemCx < 'mcx >",
                    ty_id: core::any::TypeId::of::<&'_ MemCx<'mcx>>(),
                    full_path: core::any::type_name::<&'_ MemCx<'mcx>>(),
                    module_path: {
                        let ty_name = core::any::type_name::<&'_ MemCx<'mcx>>();
                        let mut path_items: Vec<_> = ty_name.split("::").collect();
                        let _ = path_items.pop();
                        path_items.join("::")
                    },
                    composite_type: None,
                    variadic: false,
                    default: None,

                    #[doc = r" Set via the type being an `Option`."]
                    optional: false,
                    metadata: {
                        use ::pgrx::pgrx_sql_entity_graph::metadata::SqlTranslatable;
                        <&'_ MemCx<'mcx>>::entity()
                    },
                },
            },
        ])),

I believe it is this part of the expansion that is "buggy" for our purposes: this should be an anonymized or higher-rank lifetime in these cases.

@workingjubilee

Copy link
Copy Markdown
Member Author

Ah, just a missing recursion for TypeReference in anonymize_lifetimes!

Comment thread pgrx-tests/src/tests/memcxt_tests.rs Outdated
Comment thread pgrx-sql-entity-graph/src/lifetimes.rs Outdated
Comment on lines +54 to +55
// recurse
anonymize_lifetimes(&mut type_ref.elem);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the offender

Comment thread pgrx/src/memcx.rs
@workingjubilee

workingjubilee commented Nov 19, 2025

Copy link
Copy Markdown
Member Author

TODO:

  • provide more allocation functions for MemCx and PBox to cover some common cases
  • provide a few random things from Box's API that seem useful
  • move PBox to palloc module (it will eventually be joined by a PStr and PVec, in all likelihood)

@workingjubilee

workingjubilee commented Nov 21, 2025

Copy link
Copy Markdown
Member Author

I initially thought I could add more conveniences, more easily, but it actually has a bit of API design required to do well at it. My biggest add without much design effort is to offer a PBox::new_in equivalent.

Comment thread pgrx/src/palloc/pbox.rs
Comment thread pgrx/src/memcx.rs
Comment thread pgrx/src/palloc/pbox.rs Outdated
Comment thread pgrx/src/palloc/pbox.rs
Comment thread pgrx/src/palloc/pbox.rs Outdated
Comment thread pgrx/src/memcx.rs Outdated
Comment thread pgrx/src/memcx.rs
Comment thread pgrx/src/palloc/pbox.rs Outdated
Comment thread pgrx/src/memcx.rs
Comment thread pgrx/src/palloc/pbox.rs
Comment thread pgrx/src/palloc/pbox.rs Outdated
@workingjubilee

Copy link
Copy Markdown
Member Author

Oh, I should also include Deref

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the name PBox but I don't have any better ideas that aren't annoyingly long. So I'm not going to let that hold up forward progress here. feel free to merge! Thanks!

Comment thread pgrx/src/palloc/pbox.rs

[stdbox]: alloc::boxed::Box
*/
pub struct PBox<'mcx, T: ?Sized> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. I don't like the name PBox but I don't have a better suggestion. I think I don't like it because it's too similar to the existing PgBox.

Comment thread pgrx/src/memcx.rs Outdated
Comment thread pgrx/src/palloc/pbox.rs Outdated
Comment thread pgrx/src/memcx.rs
}
}

fn is_virtual_arg() -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "virtual arg"? I guess it's one we inject and not one the user explicitly listed in the #[pg_extern] function?

@workingjubilee workingjubilee Nov 22, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to be clear it's one that doesn't appear in the generated SQL: it's real to Rust, "fake" to SQL, and not a Datum.

Comment thread pgrx/src/datum/borrow.rs Outdated
Comment thread pgrx/src/palloc/pbox.rs
Comment thread pgrx/src/memcx.rs Outdated
@workingjubilee

Copy link
Copy Markdown
Member Author

The macOS failures are utterly mysterious.

@workingjubilee workingjubilee merged commit 25285e4 into pgcentralfoundation:develop Nov 22, 2025
12 of 14 checks passed
@workingjubilee workingjubilee deleted the memcx-virtual-arg branch November 22, 2025 19:32
daamien pushed a commit to daamien/pgrx that referenced this pull request Dec 15, 2025
…gcentralfoundation#2210)

tl;dr: `caller_picks_lifetime + macro_expansion == unsoundness`,
so prevent that by adding API for new, lifetime-bound allocations.

For functions exposed to PostgreSQL via `pg_extern`, signatures often
resemble these:

```rust
#[pg_extern]
fn do_something(
    floats: Array<'a, f32>,
    hms: Time,
    day: TimestampTz
) -> Vec<TimestampTz> {
```

```sql
-- pgrx will generate something like this
CREATE FUNCTION do_something(
    floats real[],
    hms time,
    day timestamptz
) RETURNS timestamptz[]
```


In SQL, we accept an array and return an array of a different type, but
in Rust, we return a `Vec`. This is because we lack API to allocate and
return a fresh `Array<'a, T>`s. Behind the scenes, we *do* allocate an
`ArrayType` at the translation boundary, unseen to a Rust programmer.
That, plus the Vec, means two allocations just to mutate an array, even
for a fixed-size type!

Unfortunately, pgrx-offered type translations like `Array<'a, T>`,
cannot have a lifetime *and* expose a new *safe* function to make it
from lifetime-free values! Such API must have the `unsafe` requirement
of access-exclusivity for its lifetime, described in functions like
`slice::from_raw_parts_mut`[^1], because the lifetime becomes
caller-selected. This makes it easy to use a `'static` lifetime with
`pg_extern` and expand `unsafe` code that trusts that lifetime.

A `'static` lifetime is always wrong for `palloc`ated types, even with
`TopMemoryContext`, since it dies whenever Postgres resets the context
and Postgres does have conditions under which it performs such a global
reset. The result is various extremely subtle soundness problems are
possible, yet the root cause lies in code that most never read. We can
solve this by denying the possibility of a caller-selected lifetime.
This will require removing some types, ultimately, but first we need an
API to migrate to.

This new API is `MemCx<'mcx, T>` and `PBox<'mcx, T>`. Importantly, there
is no *safe* route to creating `MemCx<'mcx, T>` with a caller-selected
lifetime. One can only reach `MemCx<'mcx, T>` safely as an argument:
either pass a closure to `memcx::current_context` or call a new
`pg_extern` function via SQL. Either way gives you `&MemCx<'current>`.
These let you create a lifetime-bound allocation that can live long
enough to return it.

We can already return fixed-size pass-by-reference types, so right now
`PBox` is only an optimization. It allows not putting a type on the
stack and then immediately moving it directly into a heap allocation.
More importantly, it enables writing tests, so it is important ground
work for returning de facto unsized values, as they can hide behind
`PBox` which is always `Sized`.

[^1]: https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html
eeeebbbbrrrr added a commit that referenced this pull request Feb 9, 2026
Welcome to pgrx v0.17.0. This is a new minor release that brings a bunch
of internal code refactoring, cleanup, new Postgres headers, and
additional `cargo-pgrx regress` CLI options.

As always, please install the latest `cargo-pgrx` with `cargo install
cargo-pgrx --version 0.17.0 --locked` and also update your extension
crate dependencies with `cargo pgrx upgrade`.

## What's Changed


### New Headers/Symbols

* include `access/tsmapi.h` by @usamoi in
#2155
* Include access/subtrans.h & access/commit_ts.h by @isdaniel in
#2147
* include `utils/guc_tables.h` by @usamoi in
#2243
* Include `scanner.h` in Rust bindings by @piki in
#2163
* Add `commands/publicationcmds.h` to generated `pg_sys` bindings by
@Sinjo in #2221
* Add the ClientAuthentication_hook by @daamien in
#2231
* add storage/dsm_*.h headers by @eeeebbbbrrrr in
#2244
* pg-sys: add catalog/heap.h bindings for PG18 by @charmitro in
#2150
* Add back `peer_dn` field to `Port` on `pg17..` by @workingjubilee in
#2200
* Add `repr(C)` to `struct Port` on `pg13..=16` by @workingjubilee in
#2201

### `cargo-pgrx` improvements

* Add a psql_verbosity parameter for cargo pgrx regress by @daamien in
#2230


### Code Cleanup

* Replace with let-chains in pgrx-bindgen by @workingjubilee in
#2168
* Replace with let-chains in pgrx-sql-entity-graph by @workingjubilee in
#2169
* Restore so-called needless lifetimes by @workingjubilee in
#2167
* Drop stray Postgres 12 support in cargo-pgrx by @workingjubilee in
#2173
* Remove deprecated enum##member constants by @workingjubilee in
#2170
* Remove `variadic!` macro by @workingjubilee in
#2171
* Semi-automated code cleanup by @workingjubilee in
#2189
* Deref instead of borrowing `Option<&String>` by @workingjubilee in
#2185
* Refactor schema.rs using a cargo command builder by @workingjubilee in
#2187
* Replace `impl AsRef<T>` with `&T` by @workingjubilee in
#2182
* Make `PgRelation::heap_relation` unsafe by @workingjubilee in
#2176
* Move `cargo_pgrx::env` to `cargo_pgrx::cargo` by @workingjubilee in
#2186
* Move `CargoProfile` into `cargo_pgrx::cargo` by @workingjubilee in
#2188
* Add Scalar trait for generic handling of primitive arrays by
@workingjubilee in #2153
* Set BGWORKER_SHMEM_ACCESS when building background workers by @cbandy
in #2161
* Stop referencing `pg_config` in schema generation by @workingjubilee
in #2174
* Show line number of failed asserts by @piki in
#2175
* Ghostbust `used_type.rs` a bit by @workingjubilee in
#2190
* Manually bind `static MyProcPort` and `struct Port` by @workingjubilee
in #2162
* Make datetime types import each other via `use super` by
@workingjubilee in #2198
* Migrate datetime types into `pgrx::datetime` by @workingjubilee in
#2199
* Use diagnostic attrs for ABI/SQL-related traits by @workingjubilee in
#2203
* Make `Scalar` provide `pg_sys::Oid` constant by @workingjubilee in
#2209
* Hoist varlena encoding fn by @workingjubilee in
#2208
* Recurse through `T` while anonymizing `&T` by @workingjubilee in
#2212
* `impl BorrowDatum for TimeTz` by @workingjubilee in
#2213
* Change `MemCx::alloc_bytes` to return `NonNull` by @workingjubilee in
#2214
* Add `MemCx<'current, T>` arguments and `PBox<'current, T>` returns by
@workingjubilee in #2210
* Further improve diagnostics for signatures with no SQL form by
@workingjubilee in #2215
* Add `array::Element` and `callconv::DatumPass` by @workingjubilee in
#2218
* Remove unused line in `memcxt_tests.rs` by @workingjubilee in
#2220
* Add `FlatArray<'_, T>` by @workingjubilee in
#2207
* Allow explicit handling of allocation errors by @workingjubilee in
#2226
* Place lbound before updating array len product by @workingjubilee in
#2236
* introduce `#[pg_guard(unsafe_entry_thread)]` by @eeeebbbbrrrr in
#2242


### Project Administrativa 

* Use Rust Edition 2024 by @workingjubilee in
#2165
* Remove testing for deprecated Intel Macs by @workingjubilee in
#2156
* Remove unused Cargo.lock for version-updater by @workingjubilee in
#2180
* Add rust-analyzer.toml by @workingjubilee in
#2179
* Note pgrx requires an unknown minimum Xcode version by @workingjubilee
in #2164

## New Contributors

* @isdaniel made their first contribution in
#2147
* @cbandy made their first contribution in
#2161
* @piki made their first contribution in
#2163
* @Sinjo made their first contribution in
#2221

**Full Changelog**:
v0.16.1...v0.17.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants