Skip to content

Retry on EINVAL from pthread_attr_setstacksize() #11885

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 1, 2014
Merged

Retry on EINVAL from pthread_attr_setstacksize() #11885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
464b2e2
Add libc::consts::os::posix01::PTHREAD_STACK_MIN
bnoordhuis Jan 28, 2014
b02b5cd
Retry on EINVAL from pthread_attr_setstacksize()
bnoordhuis Jan 28, 2014
431edac
Use __pthread_get_minstack() when available.
bnoordhuis Jan 29, 2014
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
26 changes: 23 additions & 3 deletions src/libstd/libc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,7 @@ pub mod consts {
pub static EDQUOT: c_int = 1133;
}
pub mod posix01 {
use libc::types::os::arch::c95::c_int;
use libc::types::os::arch::c95::{c_int, size_t};

pub static SIGTRAP : c_int = 5;

Expand Down Expand Up @@ -2228,6 +2228,17 @@ pub mod consts {
pub static PTHREAD_CREATE_JOINABLE: c_int = 0;
pub static PTHREAD_CREATE_DETACHED: c_int = 1;

#[cfg(target_os = "android")]
pub static PTHREAD_STACK_MIN: size_t = 8192;

#[cfg(target_arch = "arm", target_os = "linux")]
#[cfg(target_arch = "x86", target_os = "linux")]
#[cfg(target_arch = "x86_64", target_os = "linux")]
pub static PTHREAD_STACK_MIN: size_t = 16384;

#[cfg(target_arch = "mips", target_os = "linux")]
pub static PTHREAD_STACK_MIN: size_t = 131072;

pub static CLOCK_REALTIME: c_int = 0;
pub static CLOCK_MONOTONIC: c_int = 1;
}
Expand Down Expand Up @@ -2608,7 +2619,7 @@ pub mod consts {
pub static ELAST : c_int = 99;
}
pub mod posix01 {
use libc::types::os::arch::c95::c_int;
use libc::types::os::arch::c95::{c_int, size_t};

pub static SIGTRAP : c_int = 5;

Expand Down Expand Up @@ -2662,6 +2673,14 @@ pub mod consts {
pub static PTHREAD_CREATE_JOINABLE: c_int = 0;
pub static PTHREAD_CREATE_DETACHED: c_int = 1;

#[cfg(target_arch = "arm")]
pub static PTHREAD_STACK_MIN: size_t = 4096;

#[cfg(target_arch = "mips")]
#[cfg(target_arch = "x86")]
#[cfg(target_arch = "x86_64")]
pub static PTHREAD_STACK_MIN: size_t = 2048;

pub static CLOCK_REALTIME: c_int = 0;
pub static CLOCK_MONOTONIC: c_int = 4;
}
Expand Down Expand Up @@ -2990,7 +3009,7 @@ pub mod consts {
pub static ELAST : c_int = 106;
}
pub mod posix01 {
use libc::types::os::arch::c95::c_int;
use libc::types::os::arch::c95::{c_int, size_t};

pub static SIGTRAP : c_int = 5;

Expand Down Expand Up @@ -3043,6 +3062,7 @@ pub mod consts {

pub static PTHREAD_CREATE_JOINABLE: c_int = 1;
pub static PTHREAD_CREATE_DETACHED: c_int = 2;
pub static PTHREAD_STACK_MIN: size_t = 8192;
}
pub mod posix08 {
}
Expand Down
94 changes: 89 additions & 5 deletions src/libstd/rt/thread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,18 +145,30 @@ impl<T: Send> Drop for Thread<T> {
#[cfg(windows)]
mod imp {
use cast;
use cmp;
use libc;
use libc::types::os::arch::extra::{LPSECURITY_ATTRIBUTES, SIZE_T, BOOL,
LPVOID, DWORD, LPDWORD, HANDLE};
use ptr;
use unstable::stack::RED_ZONE;

pub type rust_thread = HANDLE;
pub type rust_thread_return = DWORD;

pub unsafe fn create(stack: uint, p: ~proc()) -> rust_thread {
let arg: *mut libc::c_void = cast::transmute(p);
CreateThread(ptr::mut_null(), stack as libc::size_t, super::thread_start,
arg, 0, ptr::mut_null())
// FIXME On UNIX, we guard against stack sizes that are too small but
// that's because pthreads enforces that stacks are at least
// PTHREAD_STACK_MIN bytes big. Windows has no such lower limit, it's
// just that below a certain threshold you can't do anything useful.
// That threshold is application and architecture-specific, however.
// For now, the only requirement is that it's big enough to hold the
// red zone. Round up to the next 64 kB because that's what the NT
// kernel does, might as well make it explicit. With the current
// 20 kB red zone, that makes for a 64 kB minimum stack.
let stack_size = (cmp::max(stack, RED_ZONE) + 0xfffe) & (-0xfffe - 1);
CreateThread(ptr::mut_null(), stack_size as libc::size_t,
super::thread_start, arg, 0, ptr::mut_null())
}

pub unsafe fn join(native: rust_thread) {
Expand Down Expand Up @@ -190,10 +202,13 @@ mod imp {
#[cfg(unix)]
mod imp {
use cast;
use libc::consts::os::posix01::PTHREAD_CREATE_JOINABLE;
use cmp;
use libc::consts::os::posix01::{PTHREAD_CREATE_JOINABLE, PTHREAD_STACK_MIN};
use libc;
use os;
use ptr;
use unstable::intrinsics;
use unstable::stack::RED_ZONE;

pub type rust_thread = libc::pthread_t;
pub type rust_thread_return = *u8;
Expand All @@ -202,11 +217,29 @@ mod imp {
let mut native: libc::pthread_t = intrinsics::uninit();
let mut attr: libc::pthread_attr_t = intrinsics::uninit();
assert_eq!(pthread_attr_init(&mut attr), 0);
assert_eq!(pthread_attr_setstacksize(&mut attr,
stack as libc::size_t), 0);
assert_eq!(pthread_attr_setdetachstate(&mut attr,
PTHREAD_CREATE_JOINABLE), 0);

// Reserve room for the red zone, the runtime's stack of last resort.
let stack_size = cmp::max(stack, RED_ZONE + __pthread_get_minstack(&attr) as uint);
Copy link
Member

Choose a reason for hiding this comment

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

I'd be a little uncomfortable going through dlopen/dlsym/dlclose for all created threads. This would mean that all native task spawns would take these hits.

I'm not entirely sure, but is there a difference between PTHREAD_STACK_MIN and __pthread_get_minstack?

That being said, I would be very comfortable merging this without fixing #6233 just yet, but I would imagine that a solution to #6233 would likely involve using dlsym to figure out if you can actually call it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be a little uncomfortable going through dlopen/dlsym/dlclose for all created threads. This would mean that all native task spawns would take these hits.

I suppose it would be possible to cache the result somewhere though that could blow up if someone gets creative with unloading libpthread.so, then loading it again..

I'm not entirely sure, but is there a difference between PTHREAD_STACK_MIN and __pthread_get_minstack?

The return value of __pthread_get_minstack() is PTHREAD_STACK_MIN plus whatever glibc needs for thread-local storage (which is not per se a fixed quantity.)

That being said, I would be very comfortable merging this without fixing #6233 just yet, but I would imagine that a solution to #6233 would likely involve using dlsym to figure out if you can actually call it.

I think so. You can't rely on __pthread_get_minstack() being there, it was added in glibc 2.15. Most distros ship older versions.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, let's merge this and then we can see how big the impact is.

match pthread_attr_setstacksize(&mut attr, stack_size as libc::size_t) {
0 => {
},
libc::EINVAL => {
// EINVAL means |stack_size| is either too small or not a
// multiple of the system page size. Because it's definitely
// >= PTHREAD_STACK_MIN, it must be an alignment issue.
// Round up to the neareast page and try again.
let page_size = os::page_size();
let stack_size = (stack_size + page_size - 1) & (-(page_size - 1) - 1);
assert_eq!(pthread_attr_setstacksize(&mut attr, stack_size as libc::size_t), 0);
},
errno => {
// This cannot really happen.
fail!("pthread_attr_setstacksize() error: {} ({})", os::last_os_error(), errno);
},
};

let arg: *libc::c_void = cast::transmute(p);
assert_eq!(pthread_create(&mut native, &attr,
super::thread_start, arg), 0);
Expand All @@ -228,6 +261,51 @@ mod imp {
#[cfg(not(target_os = "macos"), not(target_os = "android"))]
pub unsafe fn yield_now() { assert_eq!(pthread_yield(), 0); }

#[cfg(not(target_os = "linux"))]
unsafe fn __pthread_get_minstack(_: *libc::pthread_attr_t) -> libc::size_t {
libc::PTHREAD_STACK_MIN
}

// glibc >= 2.15 has a __pthread_get_minstack() function that returns
// PTHREAD_STACK_MIN plus however many bytes are needed for thread-local
// storage. We need that information to avoid blowing up when a small stack
// is created in an application with big thread-local storage requirements.
// See #6233 for rationale and details.
//
// Dynamically resolve the symbol for compatibility with older versions
// of glibc. Assumes that we've been dynamically linked to libpthread
// but that is currently always the case. Note that this means we take
// a dlopen/dlsym/dlclose hit for every new thread. Mitigating that by
// caching the symbol or the function's return value has its drawbacks:
//
// * Caching the symbol breaks when libpthread.so is reloaded because
// its address changes.
//
// * Caching the return value assumes that it's a fixed quantity.
// Not very future-proof and untrue in the presence of guard pages
// The reason __pthread_get_minstack() takes a *libc::pthread_attr_t
// as its argument is because it takes pthread_attr_setguardsize() into
// account.
//
// A better solution is to define __pthread_get_minstack() as a weak symbol
// but there is currently no way to express that in Rust code.
#[cfg(target_os = "linux")]
unsafe fn __pthread_get_minstack(attr: *libc::pthread_attr_t) -> libc::size_t {
use option::None;
use result::{Err, Ok};
use unstable::dynamic_lib;
match dynamic_lib::DynamicLibrary::open(None) {
Err(err) => fail!("DynamicLibrary::open(): {}", err),
Ok(handle) => {
match handle.symbol::<extern "C" fn(*libc::pthread_attr_t) ->
libc::size_t>("__pthread_get_minstack") {
Err(_) => libc::PTHREAD_STACK_MIN,
Ok(__pthread_get_minstack) => __pthread_get_minstack(attr),
}
}
}
}

extern {
fn pthread_create(native: *mut libc::pthread_t,
attr: *libc::pthread_attr_t,
Expand Down Expand Up @@ -262,4 +340,10 @@ mod tests {

#[test]
fn detached() { Thread::spawn(proc () {}) }

#[test]
fn small_stacks() {
assert_eq!(42, Thread::start_stack(0, proc () 42).join());
assert_eq!(42, Thread::start_stack(1, proc () 42).join());
}
}
2 changes: 1 addition & 1 deletion src/libstd/unstable/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
//! detection is not guaranteed to continue in the future. Usage of this module
//! is discouraged unless absolutely necessary.

static RED_ZONE: uint = 20 * 1024;
pub static RED_ZONE: uint = 20 * 1024;

/// This function is invoked from rust's current __morestack function. Segmented
/// stacks are currently not enabled as segmented stacks, but rather one giant
Expand Down