Allow explicit handling of allocation errors by workingjubilee · Pull Request #2226 · pgcentralfoundation/pgrx

workingjubilee · 2025-12-02T01:10:28Z

Right now, we have several fallible constructors for Postgres types. When this is an allocation error, we don't allow handling it. As I expect there to be more such fallible constructors in the future, and transient memory contexts can be arbitrarily limited in size, I think we should expose this problem in otherwise-fallible code. We can retain some infallible constructors elsewhere, though. If nothing else, this can allow better diagnostics via track_caller.

Resolves #2225

workingjubilee · 2025-12-02T01:11:30Z

Huh. I expected it to be more intrusive than this, tbh. Feels hard to argue against.

This reverts commit 283a60e.

workingjubilee · 2025-12-03T20:33:27Z

-        let ptr = unsafe { pg_sys::MemoryContextAlloc(self.ptr.as_ptr(), size).cast() };
-        // SAFETY: `pg_sys::MemoryContextAlloc` is not allowed to return NULL, it must error.
-        unsafe { NonNull::new_unchecked(ptr) }
+    pub fn alloc_bytes(&self, size: usize) -> Result<NonNull<u8>, OutOfMemory> {


I just changed the signature on this one because breaking users of this "direct allocation" API is something I already did once this cycle: it's now NonNull<u8>, not *mut u8. Also, they probably want to be able to make decisions about error-handling-or-not themselves since they're allocating raw buffers!

workingjubilee · 2025-12-03T20:34:29Z

+    #[track_caller]
    pub fn new_in(val: T, memcx: &MemCx<'mcx>) -> Self {
+        PBox::try_new_in(val, memcx).unwrap()


With track_caller, this way if it panics we point to the place someone called PBox::new_in.

workingjubilee · 2025-12-03T20:35:44Z

        let ndims = N;
        if N == 0 {
-            return Ok(FlatArray::new_empty(memcx));
+            return FlatArray::new_empty(memcx);


Making code like this simpler, even if it's subtle right now, seems like it will be an increasingly bigger win down the line.

workingjubilee · 2025-12-03T20:42:34Z

 // TODO: remove `non_exhaustive` when the errors have been worked out
 /// Errors occurring when constructing fresh arrays
 #[non_exhaustive]


I think this actually provides the full list of errors we can expect for this type, even in most future APIs.

Suggested change

/// Errors occurring when constructing fresh arrays

There's one more possible variant, maybe: if we add ways to use a type without a guaranteed-to-be-a-valid-element pg_sys::Oid. But we can apply that as a restriction to implementations of T: Element, so I don't think so?

eeeebbbbrrrr · 2025-12-04T14:11:04Z

+#[derive(Debug)]
+pub struct OutOfMemory {
+    _reserve: (),
+}
+impl OutOfMemory {
+    pub fn new() -> OutOfMemory {
+        OutOfMemory { _reserve: () }
+    }
+}
+


Is it worth it to have this be an actual Error and store the size it tried to allocate that it couldn't, and have that reported in its Display impl?

definitely a possibility. I haven't added it yet because I figure the caller usually has that information, but maybe for bubbling up purposes?

eeeebbbbrrrr · 2025-12-04T14:13:46Z

-        unsafe { NonNull::new_unchecked(ptr) }
+    pub fn alloc_bytes(&self, size: usize) -> Result<NonNull<u8>, OutOfMemory> {
+        let flags = (pg_sys::MCXT_ALLOC_NO_OOM) as i32;
+        let ptr = unsafe { pg_sys::MemoryContextAllocExtended(self.ptr.as_ptr(), size, flags) };


Should we preemptively check AllocSizeIsValid and/or AllocHugeSizeIsValid. If we don't, then MemoryContextAllocExtended can raise an ERROR.

And how do we expose MCXT_ALLOC_HUGE?

Some types will never want to use MCXT_ALLOC_HUGE, so I think we should handle it as its own alloc_extended function and have a newtyped input for the flags.

We could also skip past Extended and expose MemoryContextAllocAligned as the "most complicated alloc" function, but have alignments over the limit return a (richer) Err type... so on pg16 that lets you go up to the 2.pow(26) limit, and on pg15 and under that's 8 (well, align_of::<ffi::c_double>() technically, I believe).

eeeebbbbrrrr · 2025-12-04T14:13:57Z

-        unsafe { NonNull::new_unchecked(ptr) }
+    pub fn alloc_zeroed_bytes(&self, size: usize) -> Result<NonNull<u8>, OutOfMemory> {
+        let flags = (pg_sys::MCXT_ALLOC_NO_OOM | pg_sys::MCXT_ALLOC_ZERO) as i32;
+        let ptr = unsafe { pg_sys::MemoryContextAllocExtended(self.ptr.as_ptr(), size, flags) };


eeeebbbbrrrr · 2025-12-04T14:16:07Z

                unsafe {
-                    let list: *mut pg_sys::List = mcx.alloc_bytes(list_size).cast().as_ptr();
+                    let list: *mut pg_sys::List =
+                        mcx.alloc_bytes(list_size).unwrap().cast().as_ptr();


do we want to blindly .unwrap() here? Having the function return Result<&mut ListHead<'cx, T>, OutOfMemory> would be a little awkward but it would also indicate to the caller that it's possible it will allocate.

I need to rethink how this function works given I explicitly marked it as "unstable" as I didn't have much allocation infrastructure going at the time.

Right now, we have several fallible constructors for Postgres types. When this is an allocation error, we don't allow handling it. As I expect there to be more such fallible constructors in the future, and transient memory contexts can be arbitrarily limited in size, I think we should expose this problem in otherwise-fallible code. We can retain some infallible constructors elsewhere, though. If nothing else, this can allow better diagnostics via `track_caller`. Resolves pgcentralfoundation#2225

@usamoi

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

add OutOfMemory variant for ArrayAllocError

1c71a9f

workingjubilee added 3 commits December 1, 2025 17:17

Fix tests

283a60e

Revert "Fix tests"

8e9222a

This reverts commit 283a60e.

use try_new_in instead

177fd13

workingjubilee marked this pull request as ready for review December 3, 2025 20:24

workingjubilee changed the title ~~add OutOfMemory variant for ArrayAllocError~~ Allow explicit handling of allocation errors Dec 3, 2025

workingjubilee commented Dec 3, 2025

View reviewed changes

workingjubilee merged commit 0d8e302 into pgcentralfoundation:develop Dec 4, 2025
14 checks passed

workingjubilee deleted the oom-for-arrayallocerror branch December 4, 2025 09:20

eeeebbbbrrrr reviewed Dec 4, 2025

View reviewed changes

pgcentralfoundation deleted a comment from essence-groove Dec 10, 2025

eeeebbbbrrrr mentioned this pull request Feb 9, 2026

Update version to 0.17.0 #2245

Merged

BrewTestBot mentioned this pull request Feb 9, 2026

pgrx 0.17.0 Homebrew/homebrew-core#266565

Merged

Uh oh!

Uh oh!

Conversation

workingjubilee commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

workingjubilee Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

workingjubilee commented Dec 2, 2025 •

edited

Loading

workingjubilee Dec 3, 2025 •

edited

Loading

workingjubilee Dec 4, 2025 •

edited

Loading