Skip to content

Commit

Permalink
Check that pthread mutex initialization succeeded
Browse files Browse the repository at this point in the history
If pthread mutex initialization fails, the failure will go unnoticed unless
debug assertions are enabled. Any subsequent use of mutex will also silently
fail, since return values from lock & unlock operations are similarly checked
only through debug assertions.

In some implementations the mutex initialization requires a memory
allocation and so it does fail in practice.

Check that initialization succeeds to ensure that mutex guarantees
mutual exclusion.
  • Loading branch information
tmiasko committed Oct 20, 2020
1 parent a85e949 commit 21c29b1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 22 deletions.
4 changes: 4 additions & 0 deletions library/std/src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ where
}
}

pub fn cvt_nz(error: libc::c_int) -> crate::io::Result<()> {
if error == 0 { Ok(()) } else { Err(crate::io::Error::from_raw_os_error(error)) }
}

// On Unix-like platforms, libc::abort will unregister signal handlers
// including the SIGABRT handler, preventing the abort from being blocked, and
// fclose streams, with the side effect of flushing them so libc buffered
Expand Down
39 changes: 22 additions & 17 deletions library/std/src/sys/unix/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::cell::UnsafeCell;
use crate::mem::MaybeUninit;
use crate::sys::cvt_nz;

pub struct Mutex {
inner: UnsafeCell<libc::pthread_mutex_t>,
Expand Down Expand Up @@ -51,14 +52,11 @@ impl Mutex {
// PTHREAD_MUTEX_NORMAL which is guaranteed to deadlock if we try to
// re-lock it from the same thread, thus avoiding undefined behavior.
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
let r = libc::pthread_mutexattr_init(attr.as_mut_ptr());
debug_assert_eq!(r, 0);
let r = libc::pthread_mutexattr_settype(attr.as_mut_ptr(), libc::PTHREAD_MUTEX_NORMAL);
debug_assert_eq!(r, 0);
let r = libc::pthread_mutex_init(self.inner.get(), attr.as_ptr());
debug_assert_eq!(r, 0);
let r = libc::pthread_mutexattr_destroy(attr.as_mut_ptr());
debug_assert_eq!(r, 0);
cvt_nz(libc::pthread_mutexattr_init(attr.as_mut_ptr())).unwrap();
let attr = PthreadMutexAttr(&mut attr);
cvt_nz(libc::pthread_mutexattr_settype(attr.0.as_mut_ptr(), libc::PTHREAD_MUTEX_NORMAL))
.unwrap();
cvt_nz(libc::pthread_mutex_init(self.inner.get(), attr.0.as_ptr())).unwrap();
}
#[inline]
pub unsafe fn lock(&self) {
Expand Down Expand Up @@ -106,15 +104,11 @@ impl ReentrantMutex {

pub unsafe fn init(&self) {
let mut attr = MaybeUninit::<libc::pthread_mutexattr_t>::uninit();
let result = libc::pthread_mutexattr_init(attr.as_mut_ptr());
debug_assert_eq!(result, 0);
let result =
libc::pthread_mutexattr_settype(attr.as_mut_ptr(), libc::PTHREAD_MUTEX_RECURSIVE);
debug_assert_eq!(result, 0);
let result = libc::pthread_mutex_init(self.inner.get(), attr.as_ptr());
debug_assert_eq!(result, 0);
let result = libc::pthread_mutexattr_destroy(attr.as_mut_ptr());
debug_assert_eq!(result, 0);
cvt_nz(libc::pthread_mutexattr_init(attr.as_mut_ptr())).unwrap();
let attr = PthreadMutexAttr(&mut attr);
cvt_nz(libc::pthread_mutexattr_settype(attr.0.as_mut_ptr(), libc::PTHREAD_MUTEX_RECURSIVE))
.unwrap();
cvt_nz(libc::pthread_mutex_init(self.inner.get(), attr.0.as_ptr())).unwrap();
}

pub unsafe fn lock(&self) {
Expand All @@ -137,3 +131,14 @@ impl ReentrantMutex {
debug_assert_eq!(result, 0);
}
}

struct PthreadMutexAttr<'a>(&'a mut MaybeUninit<libc::pthread_mutexattr_t>);

impl Drop for PthreadMutexAttr<'_> {
fn drop(&mut self) {
unsafe {
let result = libc::pthread_mutexattr_destroy(self.0.as_mut_ptr());
debug_assert_eq!(result, 0);
}
}
}
6 changes: 1 addition & 5 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ impl Command {
envp: Option<&CStringArray>,
) -> io::Result<Option<Process>> {
use crate::mem::MaybeUninit;
use crate::sys;
use crate::sys::{self, cvt_nz};

if self.get_gid().is_some()
|| self.get_uid().is_some()
Expand Down Expand Up @@ -343,10 +343,6 @@ impl Command {
}
}

fn cvt_nz(error: libc::c_int) -> io::Result<()> {
if error == 0 { Ok(()) } else { Err(io::Error::from_raw_os_error(error)) }
}

unsafe {
let mut attrs = MaybeUninit::uninit();
cvt_nz(libc::posix_spawnattr_init(attrs.as_mut_ptr()))?;
Expand Down

0 comments on commit 21c29b1

Please sign in to comment.