Skip to content

Commit d98668a

Browse files
committed
auto merge of #12235 : huonw/rust/raii-lock, r=alexcrichton
- adds a `LockGuard` type returned by `.lock` and `.trylock` that unlocks the mutex in the destructor - renames `mutex::Mutex` to `StaticNativeMutex` - adds a `NativeMutex` type with a destructor - removes `LittleLock` - adds `#[must_use]` to `sync::mutex::Guard` to remind people to use it
2 parents 6b025c8 + 4668cdf commit d98668a

File tree

15 files changed

+334
-223
lines changed

15 files changed

+334
-223
lines changed

Diff for: src/libgreen/sched.rs

+15-17
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::rt::rtio::{RemoteCallback, PausableIdleCallback, Callback, EventLoop};
1515
use std::rt::task::BlockedTask;
1616
use std::rt::task::Task;
1717
use std::sync::deque;
18-
use std::unstable::mutex::Mutex;
18+
use std::unstable::mutex::NativeMutex;
1919
use std::unstable::raw;
2020

2121
use TaskState;
@@ -669,8 +669,7 @@ impl Scheduler {
669669
// is acquired here. This is the resumption points and the "bounce"
670670
// that it is referring to.
671671
unsafe {
672-
current_task.nasty_deschedule_lock.lock();
673-
current_task.nasty_deschedule_lock.unlock();
672+
let _guard = current_task.nasty_deschedule_lock.lock();
674673
}
675674
return current_task;
676675
}
@@ -765,10 +764,11 @@ impl Scheduler {
765764
// to it, but we're guaranteed that the task won't exit until we've
766765
// unlocked the lock so there's no worry of this memory going away.
767766
let cur = self.change_task_context(cur, next, |sched, mut task| {
768-
let lock: *mut Mutex = &mut task.nasty_deschedule_lock;
769-
unsafe { (*lock).lock() }
770-
f(sched, BlockedTask::block(task.swap()));
771-
unsafe { (*lock).unlock() }
767+
let lock: *mut NativeMutex = &mut task.nasty_deschedule_lock;
768+
unsafe {
769+
let _guard = (*lock).lock();
770+
f(sched, BlockedTask::block(task.swap()));
771+
}
772772
});
773773
cur.put();
774774
}
@@ -1453,8 +1453,8 @@ mod test {
14531453

14541454
#[test]
14551455
fn test_spawn_sched_blocking() {
1456-
use std::unstable::mutex::{Mutex, MUTEX_INIT};
1457-
static mut LOCK: Mutex = MUTEX_INIT;
1456+
use std::unstable::mutex::{StaticNativeMutex, NATIVE_MUTEX_INIT};
1457+
static mut LOCK: StaticNativeMutex = NATIVE_MUTEX_INIT;
14581458

14591459
// Testing that a task in one scheduler can block in foreign code
14601460
// without affecting other schedulers
@@ -1466,12 +1466,11 @@ mod test {
14661466
let mut handle = pool.spawn_sched();
14671467
handle.send(PinnedTask(pool.task(TaskOpts::new(), proc() {
14681468
unsafe {
1469-
LOCK.lock();
1469+
let mut guard = LOCK.lock();
14701470

14711471
start_ch.send(());
1472-
LOCK.wait(); // block the scheduler thread
1473-
LOCK.signal(); // let them know we have the lock
1474-
LOCK.unlock();
1472+
guard.wait(); // block the scheduler thread
1473+
guard.signal(); // let them know we have the lock
14751474
}
14761475

14771476
fin_ch.send(());
@@ -1503,10 +1502,9 @@ mod test {
15031502
child_ch.send(20);
15041503
pingpong(&parent_po, &child_ch);
15051504
unsafe {
1506-
LOCK.lock();
1507-
LOCK.signal(); // wakeup waiting scheduler
1508-
LOCK.wait(); // wait for them to grab the lock
1509-
LOCK.unlock();
1505+
let mut guard = LOCK.lock();
1506+
guard.signal(); // wakeup waiting scheduler
1507+
guard.wait(); // wait for them to grab the lock
15101508
}
15111509
})));
15121510
drop(handle);

Diff for: src/libgreen/simple.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ use std::rt::local::Local;
1717
use std::rt::rtio;
1818
use std::rt::task::{Task, BlockedTask};
1919
use std::task::TaskOpts;
20-
use std::unstable::sync::LittleLock;
20+
use std::unstable::mutex::NativeMutex;
2121

2222
struct SimpleTask {
23-
lock: LittleLock,
23+
lock: NativeMutex,
2424
awoken: bool,
2525
}
2626

@@ -59,9 +59,9 @@ impl Runtime for SimpleTask {
5959
to_wake.put_runtime(self as ~Runtime);
6060
unsafe {
6161
cast::forget(to_wake);
62-
let _l = (*me).lock.lock();
62+
let mut guard = (*me).lock.lock();
6363
(*me).awoken = true;
64-
(*me).lock.signal();
64+
guard.signal();
6565
}
6666
}
6767

@@ -83,7 +83,7 @@ impl Runtime for SimpleTask {
8383
pub fn task() -> ~Task {
8484
let mut task = ~Task::new();
8585
task.put_runtime(~SimpleTask {
86-
lock: LittleLock::new(),
86+
lock: unsafe {NativeMutex::new()},
8787
awoken: false,
8888
} as ~Runtime);
8989
return task;

Diff for: src/libgreen/task.rs

+5-12
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use std::rt::local::Local;
2525
use std::rt::rtio;
2626
use std::rt::task::{Task, BlockedTask, SendMessage};
2727
use std::task::TaskOpts;
28-
use std::unstable::mutex::Mutex;
28+
use std::unstable::mutex::NativeMutex;
2929
use std::unstable::raw;
3030

3131
use context::Context;
@@ -65,7 +65,7 @@ pub struct GreenTask {
6565
pool_id: uint,
6666

6767
// See the comments in the scheduler about why this is necessary
68-
nasty_deschedule_lock: Mutex,
68+
nasty_deschedule_lock: NativeMutex,
6969
}
7070

7171
pub enum TaskType {
@@ -163,7 +163,7 @@ impl GreenTask {
163163
task_type: task_type,
164164
sched: None,
165165
handle: None,
166-
nasty_deschedule_lock: unsafe { Mutex::new() },
166+
nasty_deschedule_lock: unsafe { NativeMutex::new() },
167167
task: Some(~Task::new()),
168168
}
169169
}
@@ -322,11 +322,10 @@ impl GreenTask {
322322
// uncontended except for when the task is rescheduled).
323323
fn reawaken_remotely(mut ~self) {
324324
unsafe {
325-
let mtx = &mut self.nasty_deschedule_lock as *mut Mutex;
325+
let mtx = &mut self.nasty_deschedule_lock as *mut NativeMutex;
326326
let handle = self.handle.get_mut_ref() as *mut SchedHandle;
327-
(*mtx).lock();
327+
let _guard = (*mtx).lock();
328328
(*handle).send(RunOnce(self));
329-
(*mtx).unlock();
330329
}
331330
}
332331
}
@@ -479,12 +478,6 @@ impl Runtime for GreenTask {
479478
fn wrap(~self) -> ~Any { self as ~Any }
480479
}
481480

482-
impl Drop for GreenTask {
483-
fn drop(&mut self) {
484-
unsafe { self.nasty_deschedule_lock.destroy(); }
485-
}
486-
}
487-
488481
#[cfg(test)]
489482
mod tests {
490483
use std::rt::Runtime;

Diff for: src/libnative/bookkeeping.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
//! The green counterpart for this is bookkeeping on sched pools.
1818
1919
use std::sync::atomics;
20-
use std::unstable::mutex::{Mutex, MUTEX_INIT};
20+
use std::unstable::mutex::{StaticNativeMutex, NATIVE_MUTEX_INIT};
2121

2222
static mut TASK_COUNT: atomics::AtomicUint = atomics::INIT_ATOMIC_UINT;
23-
static mut TASK_LOCK: Mutex = MUTEX_INIT;
23+
static mut TASK_LOCK: StaticNativeMutex = NATIVE_MUTEX_INIT;
2424

2525
pub fn increment() {
2626
let _ = unsafe { TASK_COUNT.fetch_add(1, atomics::SeqCst) };
@@ -29,9 +29,8 @@ pub fn increment() {
2929
pub fn decrement() {
3030
unsafe {
3131
if TASK_COUNT.fetch_sub(1, atomics::SeqCst) == 1 {
32-
TASK_LOCK.lock();
33-
TASK_LOCK.signal();
34-
TASK_LOCK.unlock();
32+
let mut guard = TASK_LOCK.lock();
33+
guard.signal();
3534
}
3635
}
3736
}
@@ -40,11 +39,12 @@ pub fn decrement() {
4039
/// the entry points of native programs
4140
pub fn wait_for_other_tasks() {
4241
unsafe {
43-
TASK_LOCK.lock();
44-
while TASK_COUNT.load(atomics::SeqCst) > 0 {
45-
TASK_LOCK.wait();
42+
{
43+
let mut guard = TASK_LOCK.lock();
44+
while TASK_COUNT.load(atomics::SeqCst) > 0 {
45+
guard.wait();
46+
}
4647
}
47-
TASK_LOCK.unlock();
4848
TASK_LOCK.destroy();
4949
}
5050
}

Diff for: src/libnative/io/net.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -218,19 +218,18 @@ pub fn init() {
218218
}
219219

220220
unsafe {
221-
use std::unstable::mutex::{Mutex, MUTEX_INIT};
221+
use std::unstable::mutex::{StaticNativeMutex, NATIVE_MUTEX_INIT};
222222
static mut INITIALIZED: bool = false;
223-
static mut LOCK: Mutex = MUTEX_INIT;
223+
static mut LOCK: StaticNativeMutex = NATIVE_MUTEX_INIT;
224224

225-
LOCK.lock();
225+
let _guard = LOCK.lock();
226226
if !INITIALIZED {
227227
let mut data: WSADATA = mem::init();
228228
let ret = WSAStartup(0x202, // version 2.2
229229
&mut data);
230230
assert_eq!(ret, 0);
231231
INITIALIZED = true;
232232
}
233-
LOCK.unlock();
234233
}
235234
}
236235

Diff for: src/libnative/io/timer_helper.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
2323
use std::cast;
2424
use std::rt;
25-
use std::unstable::mutex::{Mutex, MUTEX_INIT};
25+
use std::unstable::mutex::{StaticNativeMutex, NATIVE_MUTEX_INIT};
2626

2727
use bookkeeping;
2828
use io::timer::{Req, Shutdown};
@@ -37,11 +37,11 @@ static mut HELPER_CHAN: *mut Chan<Req> = 0 as *mut Chan<Req>;
3737
static mut HELPER_SIGNAL: imp::signal = 0 as imp::signal;
3838

3939
pub fn boot(helper: fn(imp::signal, Port<Req>)) {
40-
static mut LOCK: Mutex = MUTEX_INIT;
40+
static mut LOCK: StaticNativeMutex = NATIVE_MUTEX_INIT;
4141
static mut INITIALIZED: bool = false;
4242

4343
unsafe {
44-
LOCK.lock();
44+
let mut _guard = LOCK.lock();
4545
if !INITIALIZED {
4646
let (msgp, msgc) = Chan::new();
4747
// promote this to a shared channel
@@ -58,7 +58,6 @@ pub fn boot(helper: fn(imp::signal, Port<Req>)) {
5858
rt::at_exit(proc() { shutdown() });
5959
INITIALIZED = true;
6060
}
61-
LOCK.unlock();
6261
}
6362
}
6463

Diff for: src/libnative/task.rs

+9-18
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use std::rt::task::{Task, BlockedTask, SendMessage};
2222
use std::rt::thread::Thread;
2323
use std::rt;
2424
use std::task::TaskOpts;
25-
use std::unstable::mutex::Mutex;
25+
use std::unstable::mutex::NativeMutex;
2626
use std::unstable::stack;
2727

2828
use io;
@@ -40,7 +40,7 @@ pub fn new(stack_bounds: (uint, uint)) -> ~Task {
4040

4141
fn ops() -> ~Ops {
4242
~Ops {
43-
lock: unsafe { Mutex::new() },
43+
lock: unsafe { NativeMutex::new() },
4444
awoken: false,
4545
io: io::IoFactory::new(),
4646
// these *should* get overwritten
@@ -109,7 +109,7 @@ pub fn spawn_opts(opts: TaskOpts, f: proc()) {
109109
// This structure is the glue between channels and the 1:1 scheduling mode. This
110110
// structure is allocated once per task.
111111
struct Ops {
112-
lock: Mutex, // native synchronization
112+
lock: NativeMutex, // native synchronization
113113
awoken: bool, // used to prevent spurious wakeups
114114
io: io::IoFactory, // local I/O factory
115115

@@ -191,20 +191,19 @@ impl rt::Runtime for Ops {
191191
let task = BlockedTask::block(cur_task);
192192

193193
if times == 1 {
194-
(*me).lock.lock();
194+
let mut guard = (*me).lock.lock();
195195
(*me).awoken = false;
196196
match f(task) {
197197
Ok(()) => {
198198
while !(*me).awoken {
199-
(*me).lock.wait();
199+
guard.wait();
200200
}
201201
}
202202
Err(task) => { cast::forget(task.wake()); }
203203
}
204-
(*me).lock.unlock();
205204
} else {
206205
let mut iter = task.make_selectable(times);
207-
(*me).lock.lock();
206+
let mut guard = (*me).lock.lock();
208207
(*me).awoken = false;
209208
let success = iter.all(|task| {
210209
match f(task) {
@@ -216,9 +215,8 @@ impl rt::Runtime for Ops {
216215
}
217216
});
218217
while success && !(*me).awoken {
219-
(*me).lock.wait();
218+
guard.wait();
220219
}
221-
(*me).lock.unlock();
222220
}
223221
// re-acquire ownership of the task
224222
cur_task = cast::transmute::<uint, ~Task>(cur_task_dupe);
@@ -235,10 +233,9 @@ impl rt::Runtime for Ops {
235233
let me = &mut *self as *mut Ops;
236234
to_wake.put_runtime(self as ~rt::Runtime);
237235
cast::forget(to_wake);
238-
(*me).lock.lock();
236+
let mut guard = (*me).lock.lock();
239237
(*me).awoken = true;
240-
(*me).lock.signal();
241-
(*me).lock.unlock();
238+
guard.signal();
242239
}
243240
}
244241

@@ -254,12 +251,6 @@ impl rt::Runtime for Ops {
254251
}
255252
}
256253

257-
impl Drop for Ops {
258-
fn drop(&mut self) {
259-
unsafe { self.lock.destroy() }
260-
}
261-
}
262-
263254
#[cfg(test)]
264255
mod tests {
265256
use std::rt::Runtime;

Diff for: src/librustuv/queue.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
use std::cast;
2424
use std::libc::{c_void, c_int};
2525
use std::rt::task::BlockedTask;
26-
use std::unstable::sync::LittleLock;
26+
use std::unstable::mutex::NativeMutex;
2727
use std::sync::arc::UnsafeArc;
2828
use mpsc = std::sync::mpsc_queue;
2929

@@ -39,7 +39,7 @@ enum Message {
3939

4040
struct State {
4141
handle: *uvll::uv_async_t,
42-
lock: LittleLock, // see comments in async_cb for why this is needed
42+
lock: NativeMutex, // see comments in async_cb for why this is needed
4343
queue: mpsc::Queue<Message>,
4444
}
4545

@@ -112,7 +112,7 @@ impl QueuePool {
112112
let handle = UvHandle::alloc(None::<AsyncWatcher>, uvll::UV_ASYNC);
113113
let state = UnsafeArc::new(State {
114114
handle: handle,
115-
lock: LittleLock::new(),
115+
lock: unsafe {NativeMutex::new()},
116116
queue: mpsc::Queue::new(),
117117
});
118118
let q = ~QueuePool {

0 commit comments

Comments
 (0)