Evian-Zhang commented on code in PR #10932:
URL: https://github.com/apache/arrow-rs/pull/10932#discussion_r3900834817
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -224,17 +224,33 @@ impl Buffer {
if desired_capacity < self.capacity()
&& let Some(bytes) = Arc::get_mut(&mut self.data)
{
- if bytes.try_realloc(desired_capacity).is_ok() {
- // Realloc complete - update our pointer into `bytes`:
- self.ptr = if is_empty {
- bytes.as_ptr()
- } else {
- // SAFETY: we kept all elements leading up to the offset
- unsafe { bytes.as_ptr().add(offset) }
+ // Drop guard: keeps Buffer::ptr consistent with Bytes::ptr even
if a custom
+ // MemoryReservation::resize panics inside try_realloc (see
#10379).
+ struct PtrSync<'a> {
+ buffer_ptr: &'a mut *const u8,
+ bytes_ptr: *const Bytes,
+ offset: usize,
+ is_empty: bool,
+ }
+ impl Drop for PtrSync<'_> {
+ fn drop(&mut self) {
+ // SAFETY: bytes_ptr is valid while Arc<Bytes> is held by
Buffer
+ let base = unsafe { (*self.bytes_ptr).as_ptr() };
+ *self.buffer_ptr = if self.is_empty {
+ base
+ } else {
+ // SAFETY: offset is within the allocated region
+ unsafe { base.add(self.offset) }
+ };
Review Comment:
Yes this is totally valid and is the best practice to define a drop guard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
