From 75c5c04bee5928b4e9c86ca9d0d9536178083f18 Mon Sep 17 00:00:00 2001 From: jD91mZM2 <me@krake.one> Date: Sun, 14 Oct 2018 15:56:34 +0200 Subject: [PATCH] Implement a proper mutex type for future usage --- src/header/stdio/default.rs | 3 +- src/header/stdio/helpers.rs | 3 +- src/header/stdio/mod.rs | 39 +++++----- src/lib.rs | 1 + src/mutex.rs | 119 ++++++++++++++++++++++++++++++ src/platform/linux/mod.rs | 4 + src/platform/pal/mod.rs | 2 + src/platform/redox/mod.rs | 7 ++ tests/Makefile | 1 + tests/expected/stdio/mutex.stderr | 0 tests/expected/stdio/mutex.stdout | 0 tests/stdio/mutex.c | 26 +++++++ 12 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 src/mutex.rs create mode 100644 tests/expected/stdio/mutex.stderr create mode 100644 tests/expected/stdio/mutex.stdout create mode 100644 tests/stdio/mutex.c diff --git a/src/header/stdio/default.rs b/src/header/stdio/default.rs index 315d5f75..5d3fe1e0 100644 --- a/src/header/stdio/default.rs +++ b/src/header/stdio/default.rs @@ -5,6 +5,7 @@ use core::sync::atomic::AtomicBool; use fs::File; use io::LineWriter; +use mutex::Mutex; use platform::types::*; pub struct GlobalFile(UnsafeCell<FILE>); @@ -13,7 +14,7 @@ impl GlobalFile { let file = File::new(file); let writer = LineWriter::new(unsafe { file.get_ref() }); GlobalFile(UnsafeCell::new(FILE { - lock: AtomicBool::new(false), + lock: Mutex::new(()), file, flags: constants::F_PERM | flags, diff --git a/src/header/stdio/helpers.rs b/src/header/stdio/helpers.rs index 1544462f..3fff7993 100644 --- a/src/header/stdio/helpers.rs +++ b/src/header/stdio/helpers.rs @@ -7,6 +7,7 @@ use header::errno; use header::fcntl::*; use header::string::strchr; use io::LineWriter; +use mutex::Mutex; use platform::types::*; use platform; @@ -68,7 +69,7 @@ pub unsafe fn _fdopen(fd: c_int, mode: *const c_char) -> Option<*mut FILE> { let writer = LineWriter::new(file.get_ref()); Some(Box::into_raw(Box::new(FILE { - lock: AtomicBool::new(false), + lock: Mutex::new(()), file, flags, diff --git a/src/header/stdio/mod.rs b/src/header/stdio/mod.rs index 3b3de751..0653c1d6 100644 --- a/src/header/stdio/mod.rs +++ b/src/header/stdio/mod.rs @@ -17,6 +17,7 @@ use header::fcntl; use header::stdlib::mkstemp; use header::string::strlen; use io::{self, BufRead, LineWriter, SeekFrom, Read, Write}; +use mutex::Mutex; use platform::types::*; use platform::{Pal, Sys}; use platform::{errno, WriteByte}; @@ -59,7 +60,7 @@ impl<'a> DerefMut for Buffer<'a> { /// This struct gets exposed to the C API. pub struct FILE { // Can't use spin crate because *_unlocked functions are things in C :( - lock: AtomicBool, + lock: Mutex<()>, file: File, flags: c_int, @@ -148,7 +149,9 @@ impl WriteByte for FILE { } impl FILE { pub fn lock(&mut self) -> LockGuard { - flockfile(self); + unsafe { + flockfile(self); + } LockGuard(self) } } @@ -168,7 +171,9 @@ impl<'a> DerefMut for LockGuard<'a> { } impl<'a> Drop for LockGuard<'a> { fn drop(&mut self) { - funlockfile(self.0); + unsafe { + funlockfile(self.0); + } } } @@ -196,7 +201,7 @@ pub extern "C" fn cuserid(_s: *mut c_char) -> *mut c_char { #[no_mangle] pub extern "C" fn fclose(stream: *mut FILE) -> c_int { let stream = unsafe { &mut *stream }; - flockfile(stream); + unsafe { flockfile(stream); } let mut r = stream.flush().is_err(); let close = Sys::close(*stream.file) < 0; @@ -208,7 +213,7 @@ pub extern "C" fn fclose(stream: *mut FILE) -> c_int { // Reference files aren't closed on drop, so pretend to be a reference stream.file.reference = true; } else { - funlockfile(stream); + unsafe { funlockfile(stream); } } r as c_int @@ -347,10 +352,8 @@ pub extern "C" fn fileno(stream: *mut FILE) -> c_int { /// Do not call any functions other than those with the `_unlocked` postfix while the file is /// locked #[no_mangle] -pub extern "C" fn flockfile(file: *mut FILE) { - while ftrylockfile(file) != 0 { - atomic::spin_loop_hint(); - } +pub unsafe extern "C" fn flockfile(file: *mut FILE) { + (*file).lock.manual_lock(); } /// Open the file in mode `mode` @@ -427,7 +430,7 @@ pub extern "C" fn freopen( stream: &mut FILE, ) -> *mut FILE { let mut flags = unsafe { helpers::parse_mode_flags(mode) }; - flockfile(stream); + unsafe { flockfile(stream); } let _ = stream.flush(); if filename.is_null() { @@ -437,14 +440,14 @@ pub extern "C" fn freopen( } flags &= !(fcntl::O_CREAT | fcntl::O_EXCL | fcntl::O_CLOEXEC); if fcntl::sys_fcntl(*stream.file, fcntl::F_SETFL, flags) < 0 { - funlockfile(stream); + unsafe { funlockfile(stream); } fclose(stream); return ptr::null_mut(); } } else { let new = fopen(filename, mode); if new.is_null() { - funlockfile(stream); + unsafe { funlockfile(stream); } fclose(stream); return ptr::null_mut(); } @@ -454,7 +457,7 @@ pub extern "C" fn freopen( } else if Sys::dup2(*new.file, *stream.file) < 0 || fcntl::sys_fcntl(*stream.file, fcntl::F_SETFL, flags & fcntl::O_CLOEXEC) < 0 { - funlockfile(stream); + unsafe { funlockfile(stream); } fclose(new); fclose(stream); return ptr::null_mut(); @@ -462,7 +465,7 @@ pub extern "C" fn freopen( stream.flags = (stream.flags & constants::F_PERM) | new.flags; fclose(new); } - funlockfile(stream); + unsafe { funlockfile(stream); } stream } @@ -521,14 +524,14 @@ pub extern "C" fn ftello(stream: *mut FILE) -> off_t { /// Try to lock the file. Returns 0 for success, 1 for failure #[no_mangle] -pub extern "C" fn ftrylockfile(file: *mut FILE) -> c_int { - unsafe { &mut *file }.lock.compare_and_swap(false, true, Ordering::Acquire) as c_int +pub unsafe extern "C" fn ftrylockfile(file: *mut FILE) -> c_int { + if (*file).lock.manual_try_lock().is_ok() { 0 } else { 1 } } /// Unlock the file #[no_mangle] -pub extern "C" fn funlockfile(file: *mut FILE) { - unsafe { &mut *file }.lock.store(false, Ordering::Release); +pub unsafe extern "C" fn funlockfile(file: *mut FILE) { + (*file).lock.manual_unlock(); } /// Write `nitems` of size `size` from `ptr` to `stream` diff --git a/src/lib.rs b/src/lib.rs index 4319f6d7..465e1da9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,6 +41,7 @@ pub mod c_str; pub mod fs; pub mod header; pub mod io; +pub mod mutex; pub mod platform; pub mod start; diff --git a/src/mutex.rs b/src/mutex.rs new file mode 100644 index 00000000..87efc246 --- /dev/null +++ b/src/mutex.rs @@ -0,0 +1,119 @@ +use core::cell::UnsafeCell; +use core::intrinsics; +use core::ops::{Deref, DerefMut}; +use core::sync::atomic; +use platform::{Pal, Sys}; +use platform::types::*; + +pub const FUTEX_WAIT: c_int = 0; +pub const FUTEX_WAKE: c_int = 1; + +pub struct Mutex<T> { + lock: UnsafeCell<c_int>, + content: UnsafeCell<T> +} +unsafe impl<T: Send> Send for Mutex<T> {} +unsafe impl<T: Send> Sync for Mutex<T> {} +impl<T> Mutex<T> { + /// Create a new mutex + pub fn new(content: T) -> Self { + Self { + lock: UnsafeCell::new(0), + content: UnsafeCell::new(content) + } + } + + /// Tries to lock the mutex, fails if it's already locked. Manual means + /// it's up to you to unlock it after mutex. Returns the last atomic value + /// on failure. You should probably not worry about this, it's used for + /// internal optimizations. + pub unsafe fn manual_try_lock(&self) -> Result<&mut T, c_int> { + let value = intrinsics::atomic_cxchg(self.lock.get(), 0, 1).0; + if value == 0 { + return Ok(&mut *self.content.get()); + } + Err(value) + } + /// Lock the mutex, returning the inner content. After doing this, it's + /// your responsibility to unlock it after usage. Mostly useful for FFI: + /// Prefer normal .lock() where possible. + pub unsafe fn manual_lock(&self) -> &mut T { + let mut last = 0; + + // First, try spinning for really short durations: + for _ in 0..100 { + atomic::spin_loop_hint(); + last = match self.manual_try_lock() { + Ok(content) => return content, + Err(value) => value + }; + } + + // We're waiting for a longer duration, so let's employ a futex. + loop { + // If the value is 1, set it to 2 to signify that we're waiting for + // it to to send a FUTEX_WAKE on unlock. + // + // - Skip the atomic operation if the last value was 2, since it most likely hasn't changed. + // - Skip the futex wait if the atomic operation says the mutex is unlocked. + if last == 2 || intrinsics::atomic_cxchg(self.lock.get(), 1, 2).0 != 0 { + Sys::futex(self.lock.get(), FUTEX_WAIT, 2); + } + + last = match self.manual_try_lock() { + Ok(content) => return content, + Err(value) => value + }; + } + } + /// Unlock the mutex, if it's locked. + pub unsafe fn manual_unlock(&self) { + if intrinsics::atomic_xchg(self.lock.get(), 0) == 2 { + // At least one futex is up, so let's notify it + Sys::futex(self.lock.get(), FUTEX_WAKE, 1); + } + } + + /// Tries to lock the mutex and returns a guard that automatically unlocks + /// the mutex when it falls out of scope. + pub fn try_lock(&self) -> Option<MutexGuard<T>> { + unsafe { + self.manual_try_lock().ok().map(|content| MutexGuard { + mutex: self, + content + }) + } + } + /// Locks the mutex and returns a guard that automatically unlocks the + /// mutex when it falls out of scope. + pub fn lock(&self) -> MutexGuard<T> { + MutexGuard { + mutex: self, + content: unsafe { self.manual_lock() } + } + } +} + +pub struct MutexGuard<'a, T: 'a> { + mutex: &'a Mutex<T>, + content: &'a mut T +} +impl<'a, T> Deref for MutexGuard<'a, T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.content + } +} +impl<'a, T> DerefMut for MutexGuard<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.content + } +} +impl<'a, T> Drop for MutexGuard<'a, T> { + fn drop(&mut self) { + unsafe { + self.mutex.manual_unlock(); + } + } +} diff --git a/src/platform/linux/mod.rs b/src/platform/linux/mod.rs index 7d2df45b..a2df0e66 100644 --- a/src/platform/linux/mod.rs +++ b/src/platform/linux/mod.rs @@ -166,6 +166,10 @@ impl Pal for Sys { e(unsafe { syscall!(FTRUNCATE, fildes, length) }) as c_int } + fn futex(addr: *mut c_int, op: c_int, val: c_int) -> c_int { + unsafe { syscall!(FUTEX, addr, op, val, 0, 0, 0) as c_int } + } + fn futimens(fd: c_int, times: *const timespec) -> c_int { e(unsafe { syscall!(UTIMENSAT, fd, ptr::null::<c_char>(), times, 0) }) as c_int } diff --git a/src/platform/pal/mod.rs b/src/platform/pal/mod.rs index f4fdd25f..caea5473 100644 --- a/src/platform/pal/mod.rs +++ b/src/platform/pal/mod.rs @@ -57,6 +57,8 @@ pub trait Pal { fn ftruncate(fildes: c_int, length: off_t) -> c_int; + fn futex(addr: *mut c_int, op: c_int, val: c_int) -> c_int; + fn futimens(fd: c_int, times: *const timespec) -> c_int; fn utimens(path: &CStr, times: *const timespec) -> c_int; diff --git a/src/platform/redox/mod.rs b/src/platform/redox/mod.rs index fef69bff..c372a62c 100644 --- a/src/platform/redox/mod.rs +++ b/src/platform/redox/mod.rs @@ -340,6 +340,13 @@ impl Pal for Sys { e(syscall::ftruncate(fd as usize, len as usize)) as c_int } + fn futex(addr: *mut c_int, op: c_int, val: c_int) -> c_int { + match unsafe { syscall::futex(addr as *mut i32, op as usize, val as i32, 0, ptr::null_mut()) } { + Ok(success) => success as c_int, + Err(err) => -(err.errno as c_int) + } + } + fn futimens(fd: c_int, times: *const timespec) -> c_int { let times = [unsafe { redox_timespec::from(&*times) }, unsafe { redox_timespec::from(&*times.offset(1)) diff --git a/tests/Makefile b/tests/Makefile index f234a8ae..bdc7168f 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -26,6 +26,7 @@ EXPECT_BINS=\ stdio/fseek \ stdio/fwrite \ stdio/getc_unget \ + stdio/mutex \ stdio/printf \ stdio/rename \ stdio/scanf \ diff --git a/tests/expected/stdio/mutex.stderr b/tests/expected/stdio/mutex.stderr new file mode 100644 index 00000000..e69de29b diff --git a/tests/expected/stdio/mutex.stdout b/tests/expected/stdio/mutex.stdout new file mode 100644 index 00000000..e69de29b diff --git a/tests/stdio/mutex.c b/tests/stdio/mutex.c new file mode 100644 index 00000000..032ced27 --- /dev/null +++ b/tests/stdio/mutex.c @@ -0,0 +1,26 @@ +#include <stdio.h> + +int main() { + FILE* f = fopen("stdio/stdio.in", "r"); + + flockfile(f); + + // Commenting this out should cause a deadlock: + // flockfile(f); + + if (!ftrylockfile(f)) { + puts("Mutex unlocked but it shouldn't be"); + return -1; + } + funlockfile(f); + + if (ftrylockfile(f)) { + puts("Mutex locked but it shouldn't be"); + return -1; + } + if (!ftrylockfile(f)) { + puts("Mutex unlocked but it shouldn't be"); + return -1; + } + funlockfile(f); +} -- GitLab