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

Add getrlimit(2) and setrlimit(2) #879

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/sys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub mod quota;
#[cfg(any(target_os = "linux"))]
pub mod reboot;

pub mod resource;

pub mod select;

// TODO: Add support for dragonfly, freebsd, and ios/macos.
Expand Down
122 changes: 122 additions & 0 deletions src/sys/resource.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
//! Configure the process resource limits.
use std::mem;

use libc::{self, c_int, rlimit, RLIM_INFINITY};
pub use libc::rlim_t;

use {Errno, Result};

libc_enum!{
#[repr(i32)]
pub enum Resource {
Copy link
Contributor

Choose a reason for hiding this comment

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

This enum needs a doccomment as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

// POSIX
Copy link
Contributor

Choose a reason for hiding this comment

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

Please provide doc comments for every entry here to give users an understanding of what they mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

RLIMIT_AS,
RLIMIT_CORE,
RLIMIT_CPU,
RLIMIT_DATA,
RLIMIT_FSIZE,
RLIMIT_NOFILE,
RLIMIT_STACK,

// BSDs and Linux
#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux", target_os = "openbsd"))]
RLIMIT_MEMLOCK,
#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux", target_os = "openbsd"))]
RLIMIT_NPROC,
#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux", target_os = "openbsd"))]
RLIMIT_RSS,

// Android and Linux only
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_LOCKS,
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_MSGQUEUE,
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_NICE,
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_RTPRIO,
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_RTTIME,
#[cfg(any(target_os = "android", target_os = "linux"))]
RLIMIT_SIGPENDING,

// Available on some BSD
#[cfg(target_os = "freebsd")]
RLIMIT_KQUEUES,
#[cfg(target_os = "freebsd")]
RLIMIT_NPTS,
#[cfg(any(target_os = "freebsd", target_os = "dragonfly"))]
RLIMIT_SBSIZE,
#[cfg(target_os = "freebsd")]
RLIMIT_SWAP,
}
}

/// Get the current processes resource limits
///
/// A value of `None` corresponds to `RLIM_INFINITY`, which means there's no limit.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just remove the RLIM_INFINITY reference. For the user that implementation detail is completely hidden: "A value of None indicates that there's no limit."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

///
/// # Parameters
///
/// * `resource`: The [`Resource`] that we want to get the limits of.
///
/// # Examples
///
/// ```
/// # use nix::sys::resource::{getrlimit, Resource};
/// # fn main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need to declare main() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/// let (soft_limit, hard_limit) = getrlimit(Resource::RLIMIT_NOFILE).unwrap();
/// println!("current soft_limit: {:?}", soft_limit);
/// println!("current hard_limit: {:?}", hard_limit);
/// # }
/// ```
///
/// # References
///
/// [getrlimit(2)](https://linux.die.net/man/2/getrlimit)
///
/// [`Resource`]: enum.Resource.html
pub fn getrlimit(resource: Resource) -> Result<(Option<rlim_t>, Option<rlim_t>)> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a doccomment, should have an example as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

let mut rlim: rlimit = unsafe { mem::uninitialized() };
let res = unsafe { libc::getrlimit(resource as c_int, &mut rlim as *mut _) };
Errno::result(res).map(|_| {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ugly, but it works. Can you put a TODO in here to switch over to that other API you mentioned once it hits stable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

(if rlim.rlim_cur != RLIM_INFINITY { Some(rlim.rlim_cur) } else { None },
if rlim.rlim_max != RLIM_INFINITY { Some(rlim.rlim_max) } else { None })
})
}

/// Set the current processes resource limits
///
/// A value of `None` corresponds to `RLIM_INFINITY`, which means there's no limit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, don't bother mentioning RLIM_INFINITY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

///
/// # Parameters
///
/// * `resource`: The [`Resource`] that we want to set the limits of.
/// * `soft_limit`: The value that the kernel enforces for the corresponding resource.
/// * `hard_limit`: The ceiling for the soft limit. Must be lower or equal to the current hard limit
/// for non-root users.
///
/// # Examples
///
/// ```no_run
/// # use nix::sys::resource::{setrlimit, Resource};
/// # fn main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need the main here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/// let soft_limit = Some(1024);
/// let hard_limit = None;
/// setrlimit(Resource::RLIMIT_NOFILE, soft_limit, hard_limit).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a realistic use case auser might actually do? If not, we should try to find one more common to use as an example and provide a nice description of it before the codeblock.

/// # }
/// ```
///
/// # References
///
/// [setrlimit(2)](https://linux.die.net/man/2/setrlimit)
///
/// [`Resource`]: enum.Resource.html
pub fn setrlimit(resource: Resource, soft_limit: Option<rlim_t>, hard_limit: Option<rlim_t>) -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

I came in here to suggest changing this to

pub fn setrlimit<S=rlim_t, H=rlim_t>(resource: Resource, soft_limit: Option<rlim_t>, hard_limit: Option<rlim_t>) -> Result<()>
    where S: Into<Option<rlim_t>>, H: Into<Option<rlim_t>> { // ...

playground example

to take advantage for the impl<T> From<T> for Option<T> impl added in 1.12: rust-lang/rust#34828.

but it turns out we can't use default type params in functions on stable. There's a WG looking into this, so maybe we can open it up later this year. (This would allow setrlimit(RLIMIT_RSS, 1024 * 1024, None) without the Some wrapping on the first arg.)

let mut rlim: rlimit = unsafe { mem::uninitialized() };
rlim.rlim_cur = soft_limit.unwrap_or(RLIM_INFINITY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be checked to be less than rlim_max or should that be deferred to the setrlimit call?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to leave this to the kernel.

rlim.rlim_max = hard_limit.unwrap_or(RLIM_INFINITY);

let res = unsafe { libc::setrlimit(resource as c_int, &rlim as *const _) };
Errno::result(res).map(drop)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do Errno::result(res).map(|_| ()) as it's more clear about what it's actually doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be map(|_| ()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
1 change: 1 addition & 0 deletions test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ mod test_fcntl;
mod test_mq;
mod test_net;
mod test_nix_path;
mod test_resource;
mod test_poll;
mod test_pty;
#[cfg(any(target_os = "linux", target_os = "android"))]
Expand Down
35 changes: 35 additions & 0 deletions test/test_resource.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use nix::sys::resource::{Resource, getrlimit, setrlimit};

#[test]
pub fn test_resource_limits_nofile() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you summarize what this test is doing? Makes it easier to fix or modify them later.

let (soft_limit, hard_limit) = getrlimit(Resource::RLIMIT_NOFILE).unwrap();

// make sure the soft limit and hard limit are not equal
let soft_limit = match soft_limit {
Some(nofile) => Some(nofile -1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a space after the -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

None => Some(1024),
};
setrlimit(Resource::RLIMIT_NOFILE, soft_limit, hard_limit).unwrap();

let (new_soft_limit, new_hard_limit) = getrlimit(Resource::RLIMIT_NOFILE).unwrap();
assert_eq!(new_soft_limit, soft_limit);
}

#[test]
pub fn test_resource_limits_stack() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you provide some comments or a general summarizing comment here? It makes it easier for us reviewers to understand and to modify/fix them later.

let (mut soft_limit, hard_limit) = getrlimit(Resource::RLIMIT_STACK).unwrap();
let orig_limit = (soft_limit, hard_limit);
Copy link
Member

Choose a reason for hiding this comment

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

Your changes to the test do fix the previous failure, but the coverage isn't very good, since the soft and hard limits are often identical. How about setting RLIMIT_NOFILE to a hard-coded value of 1024 if the current soft limit is unlimited, or the current softlimit - 1 otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Do we really need two tests? Does the stack test provide any extra coverage over the RLIMIT_NOFILE test?


soft_limit = hard_limit;
setrlimit(Resource::RLIMIT_STACK, soft_limit, hard_limit).unwrap();

let limit2 = getrlimit(Resource::RLIMIT_STACK).unwrap();
assert_eq!(soft_limit, limit2.0);
assert_eq!(hard_limit, limit2.1);

setrlimit(Resource::RLIMIT_STACK, orig_limit.0, orig_limit.1).unwrap();

let final_limit = getrlimit(Resource::RLIMIT_STACK).unwrap();
assert_eq!(orig_limit.0, final_limit.0);
assert_eq!(orig_limit.1, final_limit.1);
}