Skip to content

fix: store uring buffers in UringInner - #380

Merged
Lzzzzzt merged 2 commits into
monoio-rs:masterfrom
ihciah:fix-drop-in-place
Feb 19, 2026
Merged

fix: store uring buffers in UringInner#380
Lzzzzzt merged 2 commits into
monoio-rs:masterfrom
ihciah:fix-drop-in-place

Conversation

@ihciah

@ihciah ihciah commented Jan 8, 2026

Copy link
Copy Markdown
Member

The driver leaked the timeout/eventfd buffers because they were allocated with Box::leak and later dropped with drop_in_place. That only runs in-place destructors and never frees the leaked allocations.

Move the buffers into UringInner so the Rc allocation keeps their addresses stable for io_uring, and use the inner fields directly when building SQEs. This removes the manual drop_in_place calls and lets Rust drop the buffers normally when the driver is dropped, avoiding the leak while preserving behavior.

This issue was found when reviewing #376. Thanks @fereidani !

The driver leaked the timeout/eventfd buffers because they were allocated
with Box::leak and later dropped with drop_in_place. That only runs
in-place destructors and never frees the leaked allocations.

Move the buffers into UringInner so the Rc allocation keeps their addresses
stable for io_uring, and use the inner fields directly when building SQEs.
This removes the manual drop_in_place calls and lets Rust drop the buffers
normally when the driver is dropped, avoiding the leak while preserving
behavior.

Copilot AI 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.

Pull request overview

This PR fixes a memory leak in the io_uring driver where timeout and eventfd buffers were allocated with Box::leak() but incorrectly freed using drop_in_place(), which only runs destructors without deallocating the leaked memory.

The fix moves these buffers from pointer fields in IoUringDriver into owned fields within UringInner, leveraging the existing Rc<UnsafeCell<UringInner>> allocation to maintain stable addresses for io_uring operations while ensuring proper cleanup through Rust's standard drop semantics.

Key changes:

  • Converted timespec from *mut Timespec to Timespec (owned value in UringInner)
  • Converted eventfd_read_dst from *mut u8 to [u8; 8] (owned array in UringInner)
  • Removed manual Box::leak() allocations and incorrect drop_in_place() calls

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Fix the Windows WSARecvMsg setup by calling size_of::<GUID>()
explicitly, which avoids casting a function item into an integer and
satisfies -D function-casts-as-integer.

Update the cargo-deny workflow to v2 so CVSS 4.0 advisories parse
correctly, and refresh deny.toml by removing deprecated keys and
mapping unmaintained to the new 'all' policy. This keeps the previous
strict behavior while silencing deprecation warnings.

@Lzzzzzt Lzzzzzt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you

@Lzzzzzt
Lzzzzzt merged commit aa54d16 into monoio-rs:master Feb 19, 2026
27 checks passed
DorianNiemiecSVRJS pushed a commit to DorianNiemiecSVRJS/monoio that referenced this pull request Feb 25, 2026
* fix: store uring buffers in UringInner

The driver leaked the timeout/eventfd buffers because they were allocated
with Box::leak and later dropped with drop_in_place. That only runs
in-place destructors and never frees the leaked allocations.

Move the buffers into UringInner so the Rc allocation keeps their addresses
stable for io_uring, and use the inner fields directly when building SQEs.
This removes the manual drop_in_place calls and lets Rust drop the buffers
normally when the driver is dropped, avoiding the leak while preserving
behavior.

* fix: address build and cargo-deny errors

Fix the Windows WSARecvMsg setup by calling size_of::<GUID>()
explicitly, which avoids casting a function item into an integer and
satisfies -D function-casts-as-integer.

Update the cargo-deny workflow to v2 so CVSS 4.0 advisories parse
correctly, and refresh deny.toml by removing deprecated keys and
mapping unmaintained to the new 'all' policy. This keeps the previous
strict behavior while silencing deprecation warnings.
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.

3 participants