Skip to content

Commit 0d8e302

Browse files
Allow explicit handling of allocation errors (pgcentralfoundation#2226)
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
1 parent 896949a commit 0d8e302

4 files changed

Lines changed: 48 additions & 22 deletions

File tree

pgrx/src/array/flat_array.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![deny(missing_docs)]
22
use crate::datum::{Array, BorrowDatum, Datum};
33
use crate::layout::{Align, Layout};
4-
use crate::memcx::MemCx;
4+
use crate::memcx::{MemCx, OutOfMemory};
55
use crate::nullable::Nullable;
66
use crate::palloc::PBox;
77
use crate::pgrx_sql_entity_graph::metadata::{
@@ -77,6 +77,8 @@ pub enum ArrayAllocError {
7777
TooManyElems,
7878
/// One or more dimensions are zero
7979
ZeroLenDim,
80+
/// There is insufficient memory in this context
81+
OutOfMemory,
8082
}
8183

8284
const MAX_ALLOC_SIZE: usize = 0x3fffffff;
@@ -104,7 +106,7 @@ where
104106

105107
let ndims = N;
106108
if N == 0 {
107-
return Ok(FlatArray::new_empty(memcx));
109+
return FlatArray::new_empty(memcx);
108110
}
109111
const { assert!(N <= MAX_DIMS) };
110112

@@ -151,7 +153,7 @@ where
151153
let elemtype = <T as Scalar>::OID;
152154
let tail_size = size - base_size;
153155

154-
let ptr = alloc_zeroed_head(memcx, tail_size, ndims as i32, dataoffset, elemtype);
156+
let ptr = alloc_zeroed_head(memcx, tail_size, ndims as i32, dataoffset, elemtype)?;
155157
// SAFETY: we allocated enough for our dimensions and lbounds
156158
unsafe {
157159
ptr.byte_add(base_size).cast().write(dim_ints);
@@ -164,11 +166,13 @@ where
164166
}
165167

166168
/// Allocate a 0-dimension array
167-
pub fn new_empty<'cx>(memcx: &MemCx<'cx>) -> PBox<'cx, FlatArray<'cx, T>> {
169+
pub fn new_empty<'cx>(
170+
memcx: &MemCx<'cx>,
171+
) -> Result<PBox<'cx, FlatArray<'cx, T>>, ArrayAllocError> {
168172
let nbytes = mem::size_of::<pg_sys::ArrayType>();
169-
let ptr = alloc_zeroed_head(memcx, 0, 0, 0, <T as Scalar>::OID);
173+
let ptr = alloc_zeroed_head(memcx, 0, 0, 0, <T as Scalar>::OID)?;
170174
// SAFETY: it's valid, if 0-dimensional
171-
unsafe { PBox::from_raw_in(FlatArray::cast_tailed(ptr), memcx) }
175+
Ok(unsafe { PBox::from_raw_in(FlatArray::cast_tailed(ptr), memcx) })
172176
}
173177

174178
/// Allocate an array sized to fit a slice and copy it
@@ -179,7 +183,7 @@ where
179183
memcx: &MemCx<'cx>,
180184
) -> Result<PBox<'cx, FlatArray<'cx, T>>, ArrayAllocError> {
181185
match data.len() {
182-
0 => Ok(FlatArray::new_empty(memcx)),
186+
0 => FlatArray::new_empty(memcx),
183187
len => {
184188
let mut array = FlatArray::new_zeroed_in([len], false, memcx)?;
185189
array.as_non_null_slice_mut().unwrap().copy_from_slice(data);
@@ -291,9 +295,9 @@ fn alloc_zeroed_head(
291295
ndim: ffi::c_int,
292296
dataoffset: i32,
293297
elemtype: pg_sys::Oid,
294-
) -> ptr::NonNull<[u8]> {
298+
) -> Result<ptr::NonNull<[u8]>, ArrayAllocError> {
295299
let nbytes = size_of::<pg_sys::ArrayType>() + tail_size;
296-
let ptr = memcx.alloc_zeroed_bytes(nbytes);
300+
let ptr = memcx.alloc_zeroed_bytes(nbytes)?;
297301
// SAFETY: We allocated at least the ArrayType's prefix of bytes
298302
unsafe {
299303
// COMPAT: write ArrayType so fields must be initialized even if ArrayType changes
@@ -305,7 +309,7 @@ fn alloc_zeroed_head(
305309
elemtype,
306310
})
307311
}
308-
ptr::NonNull::slice_from_raw_parts(ptr, tail_size)
312+
Ok(ptr::NonNull::slice_from_raw_parts(ptr, tail_size))
309313
}
310314

311315
unsafe impl<T: ?Sized> BorrowDatum for FlatArray<'_, T> {
@@ -408,3 +412,9 @@ where
408412

409413
impl<'arr, T> ExactSizeIterator for ArrayIter<'arr, T> where T: ?Sized + Element {}
410414
impl<'arr, T> FusedIterator for ArrayIter<'arr, T> where T: ?Sized + Element {}
415+
416+
impl From<OutOfMemory> for ArrayAllocError {
417+
fn from(value: OutOfMemory) -> Self {
418+
ArrayAllocError::OutOfMemory
419+
}
420+
}

pgrx/src/list/flat_list.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ impl<'cx, T: Enlist> List<'cx, T> {
112112
// No silly reasoning, simply allocate ~2 cache lines for a list
113113
let list_size = 128;
114114
unsafe {
115-
let list: *mut pg_sys::List = mcx.alloc_bytes(list_size).cast().as_ptr();
115+
let list: *mut pg_sys::List =
116+
mcx.alloc_bytes(list_size).unwrap().cast().as_ptr();
116117
assert!(list.is_non_null());
117118
(*list).type_ = T::LIST_TAG;
118119
(*list).max_length = ((list_size - mem::size_of::<pg_sys::List>())

pgrx/src/memcx.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,18 @@ impl<'mcx> MemCx<'mcx> {
3636

3737
/// Allocate a raw byte buffer `size` bytes in length
3838
/// and returns a pointer to the new allocation.
39-
pub fn alloc_bytes(&self, size: usize) -> NonNull<u8> {
40-
let ptr = unsafe { pg_sys::MemoryContextAlloc(self.ptr.as_ptr(), size).cast() };
41-
// SAFETY: `pg_sys::MemoryContextAlloc` is not allowed to return NULL, it must error.
42-
unsafe { NonNull::new_unchecked(ptr) }
39+
pub fn alloc_bytes(&self, size: usize) -> Result<NonNull<u8>, OutOfMemory> {
40+
let flags = (pg_sys::MCXT_ALLOC_NO_OOM) as i32;
41+
let ptr = unsafe { pg_sys::MemoryContextAllocExtended(self.ptr.as_ptr(), size, flags) };
42+
NonNull::new(ptr.cast()).ok_or(OutOfMemory::new())
4343
}
4444

4545
/// Allocate a raw byte buffer `size` bytes in length
4646
/// and returns a pointer to the new allocation.
47-
pub fn alloc_zeroed_bytes(&self, size: usize) -> NonNull<u8> {
48-
let ptr = unsafe { pg_sys::MemoryContextAllocZero(self.ptr.as_ptr(), size).cast() };
49-
// SAFETY: `pg_sys::MemoryContextAlloc` is not allowed to return NULL, it must error.
50-
unsafe { NonNull::new_unchecked(ptr) }
47+
pub fn alloc_zeroed_bytes(&self, size: usize) -> Result<NonNull<u8>, OutOfMemory> {
48+
let flags = (pg_sys::MCXT_ALLOC_NO_OOM | pg_sys::MCXT_ALLOC_ZERO) as i32;
49+
let ptr = unsafe { pg_sys::MemoryContextAllocExtended(self.ptr.as_ptr(), size, flags) };
50+
NonNull::new(ptr.cast()).ok_or(OutOfMemory::new())
5151
}
5252

5353
/// Stores the current memory context, switches to *this* memory context,
@@ -68,6 +68,16 @@ impl<'mcx> MemCx<'mcx> {
6868
}
6969
}
7070

71+
#[derive(Debug)]
72+
pub struct OutOfMemory {
73+
_reserve: (),
74+
}
75+
impl OutOfMemory {
76+
pub fn new() -> OutOfMemory {
77+
OutOfMemory { _reserve: () }
78+
}
79+
}
80+
7181
/// Acquire the current context and operate inside it.
7282
pub fn current_context<'curr, F, T>(f: F) -> T
7383
where

pgrx/src/palloc/pbox.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::callconv::{BoxRet, FcInfo};
22
use crate::datum::{BorrowDatum, Datum};
33
use crate::layout::PassBy;
4-
use crate::memcx::MemCx;
4+
use crate::memcx::{MemCx, OutOfMemory};
55
use crate::pg_sys;
66
use core::marker::PhantomData;
77
use core::ops::{Deref, DerefMut};
@@ -33,13 +33,18 @@ impl<'mcx, T: ?Sized> PBox<'mcx, T> {
3333
}
3434

3535
impl<'mcx, T: Sized> PBox<'mcx, T> {
36+
#[track_caller]
3637
pub fn new_in(val: T, memcx: &MemCx<'mcx>) -> Self {
38+
PBox::try_new_in(val, memcx).unwrap()
39+
}
40+
41+
pub fn try_new_in(val: T, memcx: &MemCx<'mcx>) -> Result<Self, OutOfMemory> {
3742
const { assert!(align_of::<T>() <= size_of::<pg_sys::Datum>()) };
38-
let ptr = memcx.alloc_bytes(size_of::<T>()).cast();
43+
let ptr = memcx.alloc_bytes(size_of::<T>())?.cast();
3944
// SAFETY: We were guaranteed an appropriately sized allocation to write to,
4045
// and we have asserted our alignment maximum was upheld
4146
unsafe { ptr.write(val) };
42-
PBox { ptr, _cx: PhantomData }
47+
Ok(PBox { ptr, _cx: PhantomData })
4348
}
4449
}
4550

0 commit comments

Comments
 (0)