Skip to content

Commit e156d00

Browse files
committed
rustrt: Allow dropping a brand-new Task
When a new task fails to spawn, it triggers a task failure of the spawning task. This ends up causing runtime aborts today because of the destructor bomb in the Task structure. The bomb doesn't actually need to go off until *after* the task has run at least once. This now prevents a runtime abort when a native thread fails to spawn.
1 parent 692077b commit e156d00

File tree

3 files changed

+33
-10
lines changed

3 files changed

+33
-10
lines changed

src/libgreen/sched.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ impl Scheduler {
219219
let message = stask.sched.get_mut_ref().message_queue.pop();
220220
rtassert!(match message { msgq::Empty => true, _ => false });
221221

222-
stask.task.get_mut_ref().destroyed = true;
222+
stask.task.take().unwrap().drop();
223223
}
224224

225225
// This does not return a scheduler, as the scheduler is placed

src/librustrt/local.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ mod test {
125125
}).join();
126126
}
127127

128-
fn cleanup_task(mut t: Box<Task>) {
129-
t.destroyed = true;
128+
fn cleanup_task(t: Box<Task>) {
129+
t.drop();
130130
}
131131

132132
}

src/librustrt/task.rs

+30-7
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,21 @@ pub struct Task {
100100
pub storage: LocalStorage,
101101
pub unwinder: Unwinder,
102102
pub death: Death,
103-
pub destroyed: bool,
104103
pub name: Option<SendStr>,
105104

105+
state: TaskState,
106106
imp: Option<Box<Runtime + Send>>,
107107
}
108108

109+
// Once a task has entered the `Armed` state it must be destroyed via `drop`,
110+
// and no other method. This state is used to track this transition.
111+
#[deriving(PartialEq)]
112+
enum TaskState {
113+
New,
114+
Armed,
115+
Destroyed,
116+
}
117+
109118
pub struct TaskOpts {
110119
/// Invoke this procedure with the result of the task when it finishes.
111120
pub on_exit: Option<proc(Result): Send>,
@@ -159,7 +168,7 @@ impl Task {
159168
storage: LocalStorage(None),
160169
unwinder: Unwinder::new(),
161170
death: Death::new(),
162-
destroyed: false,
171+
state: New,
163172
name: None,
164173
imp: None,
165174
}
@@ -203,7 +212,7 @@ impl Task {
203212
/// }).destroy();
204213
/// # }
205214
/// ```
206-
pub fn run(self: Box<Task>, f: ||) -> Box<Task> {
215+
pub fn run(mut self: Box<Task>, f: ||) -> Box<Task> {
207216
assert!(!self.is_destroyed(), "cannot re-use a destroyed task");
208217

209218
// First, make sure that no one else is in TLS. This does not allow
@@ -212,6 +221,7 @@ impl Task {
212221
if Local::exists(None::<Task>) {
213222
fail!("cannot run a task recursively inside another");
214223
}
224+
self.state = Armed;
215225
Local::put(self);
216226

217227
// There are two primary reasons that general try/catch is unsafe. The
@@ -333,12 +343,12 @@ impl Task {
333343
// Now that we're done, we remove the task from TLS and flag it for
334344
// destruction.
335345
let mut task: Box<Task> = Local::take();
336-
task.destroyed = true;
346+
task.state = Destroyed;
337347
return task;
338348
}
339349

340350
/// Queries whether this can be destroyed or not.
341-
pub fn is_destroyed(&self) -> bool { self.destroyed }
351+
pub fn is_destroyed(&self) -> bool { self.state == Destroyed }
342352

343353
/// Inserts a runtime object into this task, transferring ownership to the
344354
/// task. It is illegal to replace a previous runtime object in this task
@@ -453,12 +463,20 @@ impl Task {
453463
pub fn can_block(&self) -> bool {
454464
self.imp.get_ref().can_block()
455465
}
466+
467+
/// Consume this task, flagging it as a candidate for destruction.
468+
///
469+
/// This function is required to be invoked to destroy a task. A task
470+
/// destroyed through a normal drop will abort.
471+
pub fn drop(mut self) {
472+
self.state = Destroyed;
473+
}
456474
}
457475

458476
impl Drop for Task {
459477
fn drop(&mut self) {
460478
rtdebug!("called drop for a task: {}", self as *mut Task as uint);
461-
rtassert!(self.destroyed);
479+
rtassert!(self.state != Armed);
462480
}
463481
}
464482

@@ -634,12 +652,17 @@ mod test {
634652
begin_unwind("cause", file!(), line!())
635653
}
636654

655+
#[test]
656+
fn drop_new_task_ok() {
657+
drop(Task::new());
658+
}
659+
637660
// Task blocking tests
638661

639662
#[test]
640663
fn block_and_wake() {
641664
let task = box Task::new();
642665
let mut task = BlockedTask::block(task).wake().unwrap();
643-
task.destroyed = true;
666+
task.destroy();
644667
}
645668
}

0 commit comments

Comments
 (0)