Skip to content

Commit

Permalink
[libra-infallible] std::sync::RwLock -> wrapped RwLock
Browse files Browse the repository at this point in the history
  • Loading branch information
mimoo authored and bors-libra committed Oct 9, 2020
1 parent ee2db72 commit dc70234
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 3 deletions.
2 changes: 2 additions & 0 deletions common/infallible/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// SPDX-License-Identifier: Apache-2.0

mod mutex;
mod rwlock;
mod time;

pub use mutex::Mutex;
pub use rwlock::RwLock;
pub use time::duration_since_epoch;
2 changes: 1 addition & 1 deletion common/infallible/src/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ mod tests {
let _ = thread1.join();
let _ = thread2.join();

println!("{:?}", *mutex.lock());
let _ = mutex.lock();
}
}
66 changes: 66 additions & 0 deletions common/infallible/src/rwlock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) The Libra Core Contributors
// SPDX-License-Identifier: Apache-2.0

use std::sync::{RwLock as RwLockImpl, RwLockReadGuard, RwLockWriteGuard};

/// A simple wrapper around the lock() function of a std::sync::RwLock
/// The only difference is that you don't need to call unwrap() on it.
#[derive(Debug, Default)]
pub struct RwLock<T>(RwLockImpl<T>);

impl<T> RwLock<T> {
/// creates a read-write lock
pub fn new(t: T) -> Self {
Self(RwLockImpl::new(t))
}

/// lock the rwlock in read mode
pub fn read(&self) -> RwLockReadGuard<'_, T> {
self.0
.read()
.expect("libra cannot currently handle a poisoned lock")
}

/// lock the rwlock in write mode
pub fn write(&self) -> RwLockWriteGuard<'_, T> {
self.0
.write()
.expect("libra cannot currently handle a poisoned lock")
}

/// return the owned type consuming the lock
pub fn into_inner(self) -> T {
self.0
.into_inner()
.expect("libra cannot currently handle a poisoned lock")
}
}

#[cfg(test)]
mod tests {

use super::*;
use std::{sync::Arc, thread};

#[test]
fn test_libra_rwlock() {
let a = 7u8;
let rwlock = Arc::new(RwLock::new(a));
let rwlock2 = rwlock.clone();
let rwlock3 = rwlock.clone();

let thread1 = thread::spawn(move || {
let mut b = rwlock2.write();
*b = 8;
});
let thread2 = thread::spawn(move || {
let mut b = rwlock3.write();
*b = 9;
});

let _ = thread1.join();
let _ = thread2.join();

let _ = rwlock.read();
}
}
9 changes: 7 additions & 2 deletions documentation/coding_guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,16 @@ Listed below are high-level suggestions based on experience:

Error handling suggestions follow the [Rust book guidance](https://doc.rust-lang.org/book/ch09-00-error-handling.html). Rust groups errors into two major categories: recoverable and unrecoverable errors. Recoverable errors should be handled with [Result](https://doc.rust-lang.org/std/result/). Our suggestions on unrecoverable errors are listed below:

*Fallible functions*

* `duration_since_epoch()` - to obtain the unix time, call the function provided by `libra-infallible`.
* `RwLock` and `Mutex` - Instead of calling `unwrap()` on the standard library implementations of these functions, use the infallible equivalent types that we provide in `libra-infallible`.

*Panic*

* `unwrap()` - Unwrap should only be used for mutexes (e.g. `lock().unwrap()`) and test code. For all other use cases, prefer `expect()`. The only exception is if the error message is custom-generated, in which case use `.unwrap_or_else(|| panic!("error: {}", foo))`
* `unwrap()` - Unwrap should only be used for test code. For all other use cases, prefer `expect()`. The only exception is if the error message is custom-generated, in which case use `.unwrap_or_else(|| panic!("error: {}", foo))`.
* `expect()` - Expect should be invoked when a system invariant is expected to be preserved. `expect()` is preferred over `unwrap()` and should contain a detailed error message on failure in most cases.
* `assert!()` - This macro is kept in both debug/release and should be used to protect invariants of the system as necessary
* `assert!()` - This macro is kept in both debug/release and should be used to protect invariants of the system as necessary.
* `unreachable!()` - This macro will panic on code that should not be reached (violating an invariant) and can be used where appropriate.

In production (non-test) code, outside of lock management, all unrecoverable errors should be cleanly documented describing why said event is unrecoverable. For example, if the system is now in a bad state, state what that state is and the motivation for why a crash / restart is more effective than resolving it within a running system, and what if any steps an operator would need to take to resolve the issue.
Expand Down

0 comments on commit dc70234

Please sign in to comment.