Skip to content

Commit

Permalink
Use __pthread_get_minstack() when available.
Browse files Browse the repository at this point in the history
glibc >= 2.15 has a __pthread_get_minstack() function that returns
PTHREAD_STACK_MIN plus however many bytes are needed for thread-local
storage.  Use it when it's available because just PTHREAD_STACK_MIN is
not enough in applications that have big thread-local storage
requirements.

Fixes #6233.
  • Loading branch information
bnoordhuis committed Jan 31, 2014
1 parent b02b5cd commit 431edac
Showing 1 changed file with 46 additions and 1 deletion.
47 changes: 46 additions & 1 deletion src/libstd/rt/thread.rs
Expand Up @@ -221,7 +221,7 @@ mod imp {
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_STACK_MIN as uint);
let stack_size = cmp::max(stack, RED_ZONE + __pthread_get_minstack(&attr) as uint);
match pthread_attr_setstacksize(&mut attr, stack_size as libc::size_t) {
0 => {
},
Expand Down Expand Up @@ -261,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

12 comments on commit 431edac

@bors
Copy link
Contributor

@bors bors commented on 431edac Jan 31, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 431edac Jan 31, 2014

Choose a reason for hiding this comment

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

merging bnoordhuis/rust/issue11694 = 431edac into auto

@bors
Copy link
Contributor

@bors bors commented on 431edac Jan 31, 2014

Choose a reason for hiding this comment

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

bnoordhuis/rust/issue11694 = 431edac merged ok, testing candidate = 22174591

@bors
Copy link
Contributor

@bors bors commented on 431edac Jan 31, 2014

@bnoordhuis
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'm unclear on why the CI is failing. Is it hitting some kind of timeout? The build logs all end with the same error when running the tests. FWIW, they pass for me locally on OS X and Linux.

compile_and_link: i686-unknown-linux-gnu/stage2/test/rustctest-i686-unknown-linux-gnu

command interrupted, attempting to kill

command interrupted, attempting to kill

command interrupted, attempting to kill

command interrupted, attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1217.360580

@alexcrichton
Copy link
Member

Choose a reason for hiding this comment

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

This is something that's in general plaguing the mac64 bots right now, I'm trying to investigate.

@alexcrichton
Copy link
Member

Choose a reason for hiding this comment

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

My current suspicions point to cb263e8, trying to confirm right now.

@bors
Copy link
Contributor

@bors bors commented on 431edac Feb 1, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 431edac Feb 1, 2014

Choose a reason for hiding this comment

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

merging bnoordhuis/rust/issue11694 = 431edac into auto

@bors
Copy link
Contributor

@bors bors commented on 431edac Feb 1, 2014

Choose a reason for hiding this comment

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

bnoordhuis/rust/issue11694 = 431edac merged ok, testing candidate = a1f157b

@bors
Copy link
Contributor

@bors bors commented on 431edac Feb 1, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 431edac Feb 1, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = a1f157b

Please sign in to comment.