Skip to content

Commit cd92d2c

Browse files
committed
Make rekillable consistent with unkillable
As for now, rekillable is an unsafe function, instead, it should behave just like unkillable by encapsulating unsafe code within an unsafe block. This patch does that and removes unsafe blocks that were encapsulating rekillable calls throughout rust's libs. Fixes #8232
1 parent 9cd91c8 commit cd92d2c

File tree

2 files changed

+46
-45
lines changed

2 files changed

+46
-45
lines changed

src/libextra/sync.rs

+19-29
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,7 @@ impl<Q:Send> Sem<Q> {
135135
do task::unkillable {
136136
do (|| {
137137
self.acquire();
138-
unsafe {
139-
do task::rekillable { blk() }
140-
}
138+
do task::rekillable { blk() }
141139
}).finally {
142140
self.release();
143141
}
@@ -234,10 +232,8 @@ impl<'self> Condvar<'self> {
234232
// signaller already sent -- I mean 'unconditionally' in contrast
235233
// with acquire().)
236234
do (|| {
237-
unsafe {
238-
do task::rekillable {
239-
let _ = WaitEnd.take_unwrap().recv();
240-
}
235+
do task::rekillable {
236+
let _ = WaitEnd.take_unwrap().recv();
241237
}
242238
}).finally {
243239
// Reacquire the condvar. Note this is back in the unkillable
@@ -516,14 +512,12 @@ impl RWLock {
516512
* 'write' from other tasks will run concurrently with this one.
517513
*/
518514
pub fn write<U>(&self, blk: &fn() -> U) -> U {
519-
unsafe {
520-
do task::unkillable {
521-
(&self.order_lock).acquire();
522-
do (&self.access_lock).access {
523-
(&self.order_lock).release();
524-
do task::rekillable {
525-
blk()
526-
}
515+
do task::unkillable {
516+
(&self.order_lock).acquire();
517+
do (&self.access_lock).access {
518+
(&self.order_lock).release();
519+
do task::rekillable {
520+
blk()
527521
}
528522
}
529523
}
@@ -562,16 +556,14 @@ impl RWLock {
562556
// which can't happen until T2 finishes the downgrade-read entirely.
563557
// The astute reader will also note that making waking writers use the
564558
// order_lock is better for not starving readers.
565-
unsafe {
566-
do task::unkillable {
567-
(&self.order_lock).acquire();
568-
do (&self.access_lock).access_cond |cond| {
569-
(&self.order_lock).release();
570-
do task::rekillable {
571-
let opt_lock = Just(&self.order_lock);
572-
blk(&Condvar { sem: cond.sem, order: opt_lock,
573-
token: NonCopyable::new() })
574-
}
559+
do task::unkillable {
560+
(&self.order_lock).acquire();
561+
do (&self.access_lock).access_cond |cond| {
562+
(&self.order_lock).release();
563+
do task::rekillable {
564+
let opt_lock = Just(&self.order_lock);
565+
blk(&Condvar { sem: cond.sem, order: opt_lock,
566+
token: NonCopyable::new() })
575567
}
576568
}
577569
}
@@ -606,10 +598,8 @@ impl RWLock {
606598
(&self.access_lock).acquire();
607599
(&self.order_lock).release();
608600
do (|| {
609-
unsafe {
610-
do task::rekillable {
611-
blk(RWLockWriteMode { lock: self, token: NonCopyable::new() })
612-
}
601+
do task::rekillable {
602+
blk(RWLockWriteMode { lock: self, token: NonCopyable::new() })
613603
}
614604
}).finally {
615605
let writer_or_last_reader;

src/libstd/task/mod.rs

+27-16
Original file line numberDiff line numberDiff line change
@@ -597,21 +597,34 @@ pub fn unkillable<U>(f: &fn() -> U) -> U {
597597
}
598598
}
599599

600-
/// The inverse of unkillable. Only ever to be used nested in unkillable().
601-
pub unsafe fn rekillable<U>(f: &fn() -> U) -> U {
600+
/**
601+
* Makes killable a task marked as unkillable
602+
*
603+
* # Example
604+
*
605+
* ~~~
606+
* do task::unkillable {
607+
* do task::rekillable {
608+
* // Task is killable
609+
* }
610+
* }
611+
*/
612+
pub fn rekillable<U>(f: &fn() -> U) -> U {
602613
use rt::task::Task;
603614

604-
if in_green_task_context() {
605-
let t = Local::unsafe_borrow::<Task>();
606-
do (|| {
607-
(*t).death.allow_kill((*t).unwinder.unwinding);
615+
unsafe {
616+
if in_green_task_context() {
617+
let t = Local::unsafe_borrow::<Task>();
618+
do (|| {
619+
(*t).death.allow_kill((*t).unwinder.unwinding);
620+
f()
621+
}).finally {
622+
(*t).death.inhibit_kill((*t).unwinder.unwinding);
623+
}
624+
} else {
625+
// FIXME(#3095): As in unkillable().
608626
f()
609-
}).finally {
610-
(*t).death.inhibit_kill((*t).unwinder.unwinding);
611627
}
612-
} else {
613-
// FIXME(#3095): As in unkillable().
614-
f()
615628
}
616629
}
617630

@@ -646,11 +659,9 @@ fn test_kill_rekillable_task() {
646659
do run_in_newsched_task {
647660
do task::try {
648661
do task::unkillable {
649-
unsafe {
650-
do task::rekillable {
651-
do task::spawn {
652-
fail!();
653-
}
662+
do task::rekillable {
663+
do task::spawn {
664+
fail!();
654665
}
655666
}
656667
}

0 commit comments

Comments
 (0)