Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make UnixSteam generic on the queue size #55

Open
sbosnick opened this issue Oct 8, 2022 · 1 comment
Open

Make UnixSteam generic on the queue size #55

sbosnick opened this issue Oct 8, 2022 · 1 comment
Assignees

Comments

@sbosnick
Copy link
Member

sbosnick commented Oct 8, 2022

The various UnixStream implementations currently use a fixed size for the internal queue of file descriptors. This is current fixed at 2, though there is no particularly good reasons for this value.

The constrains on the size of the internal queue are as follows:

  1. it should be fairly small (at least by default) because it is used to calculate the size of an array allocated on the stack
  2. it should be the same on both sides of the "fd passing" (assuming both processes use this library)

The reason for the second constraint is to try to avoid the control data being truncated in the underlying call to recvmsg. This crate treats this occurrence as an error though the call to recvmsg implements it as a flag set on the msghdr structure passed as a parameter.

The second constraint also makes it a breaking change to increment the current fixed value across the board because you could end up with the sending side using a larger fixed value than the receiving side which would risk errors due to the control data being truncated on the underlying call to recvmsg.

The proposed (and non-breaking) change to allow users of the crate to have a larger (though still fixed) queue size is to make the various UnixStream implementations rely on a const generic parameter for the queue size (which would default to the current 2 to keep the change non-breaking). The proposed interface is:

pub const DEFAULT_FD_QUEUE_SIZE: usize = 2;

pub struct UnixStream<const N: usize = DEFAULT_FD_QUEUE_SIZE> { ... }

impl<const N:usize> UnixStream<N> {
    pub const FD_QUEUE_SIZE: usize = N;

    ...
}

It may be desirable to increase DEFAULT_FD_QUEUE_SIZE for the next major release but for now we would keep it at 2 to avoid a breaking change.

@sbosnick
Copy link
Member Author

The design presented above leads to an error on stable Rust with the use of the generic const parameter N in the calculation needed for the stack array size (which ultimately ends with a call to the now const fn libc::CMSG_SPACE). This error goes away with the incompletely implement #![feature(generic_const_exprs)] on nightly so it may eventually be possible to use a stack array for this purpose.

As an interim solution (until #![feature(generic_const_exprs)] is stabilized) I will try a solution using smallvec instead of a stack array.

@sbosnick sbosnick self-assigned this Oct 10, 2022
sbosnick added a commit that referenced this issue Oct 10, 2022
Add a SIZE generic const to BiQueue which defaults to
DEFAULT_FD_QUEUE_SIZE (which is set to 2). This SIZE is used for the
length of the inbound and outbound queues of fd's. It is also used for
the size of the CMsgBuffer used as a part of the calls to recvmsg and
sendmsg wrapped through MsgHdr.

The value of DEFAULT_FD_QUEUE_SIZE is chosen to match the old value of
BiQueue::FD_QUEUE_SIZE to prevent a breaking change.

Currently CMsgBuffer is a SmallVec with the size of the inline part
of the vector based on DEFAULT_FD_QUEUE_SIZE (and not on SIZE).
Current limitations in Rust prevent basing this on SIZE (though that
may change if #![feature(generic_const_exprs)] is stabalized).

This is part of #55.
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

No branches or pull requests

1 participant