Skip to content

Commit 8e96621

Browse files
committed
core::rt: Restructure task_from_last_cleanup_job to borrow correctly
We need a number of mutable references to contexts so name it `get_contexts` and return a tuple of all of them.
1 parent cf34b31 commit 8e96621

File tree

1 file changed

+63
-48
lines changed

1 file changed

+63
-48
lines changed

src/libcore/rt/sched.rs

Lines changed: 63 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,16 @@ pub impl Scheduler {
151151

152152
// Store the task in the scheduler so it can be grabbed later
153153
self.current_task = Some(task);
154-
self.swap_in_task();
154+
155+
// Take pointers to both the task and scheduler's saved registers.
156+
{
157+
let (sched_context, _, next_task_context) = self.get_contexts();
158+
let next_task_context = next_task_context.unwrap();
159+
// Context switch to the task, restoring it's registers
160+
// and saving the scheduler's
161+
Context::swap(sched_context, next_task_context);
162+
}
163+
155164
// The running task should have passed ownership elsewhere
156165
assert!(self.current_task.is_none());
157166

@@ -171,8 +180,11 @@ pub impl Scheduler {
171180

172181
let dead_task = self.current_task.swap_unwrap();
173182
self.enqueue_cleanup_job(RecycleTask(dead_task));
174-
let dead_task = self.task_from_last_cleanup_job();
175-
self.swap_out_task(dead_task);
183+
{
184+
let (sched_context, last_task_context, _) = self.get_contexts();
185+
let last_task_context = last_task_context.unwrap();
186+
Context::swap(last_task_context, sched_context);
187+
}
176188
}
177189

178190
/// Block a running task, context switch to the scheduler, then pass the
@@ -194,9 +206,13 @@ pub impl Scheduler {
194206
};
195207
let f_opaque = HackAroundBorrowCk::from_fn(f_fake_region);
196208
self.enqueue_cleanup_job(GiveTask(blocked_task, f_opaque));
197-
let blocked_task = self.task_from_last_cleanup_job();
209+
{
210+
let (sched_context, last_task_context, _) = self.get_contexts();
211+
let last_task_context = last_task_context.unwrap();
212+
Context::swap(last_task_context, sched_context);
213+
}
198214

199-
self.swap_out_task(blocked_task);
215+
// XXX: Should probably run cleanup jobs
200216
}
201217

202218
/// Switch directly to another task, without going through the scheduler.
@@ -209,43 +225,17 @@ pub impl Scheduler {
209225

210226
let old_running_task = self.current_task.swap_unwrap();
211227
self.enqueue_cleanup_job(RescheduleTask(old_running_task));
212-
let old_running_task = self.task_from_last_cleanup_job();
213-
214228
self.current_task = Some(next_task);
215-
self.swap_in_task_from_running_task(old_running_task);
216-
}
217-
218-
219-
// * Context switching
220-
221-
// NB: When switching to a task callers are expected to first set
222-
// self.running_task. When switching away from a task likewise move
223-
// out of the self.running_task
224-
225-
priv fn swap_in_task(&mut self) {
226-
// Take pointers to both the task and scheduler's saved registers.
227-
let running_task: &~Task = self.current_task.get_ref();
228-
let task_context = &running_task.saved_context;
229-
let scheduler_context = &mut self.saved_context;
230-
231-
// Context switch to the task, restoring it's registers
232-
// and saving the scheduler's
233-
Context::swap(scheduler_context, task_context);
234-
}
235-
236-
priv fn swap_out_task(&mut self, running_task: &mut Task) {
237-
let task_context = &mut running_task.saved_context;
238-
let scheduler_context = &self.saved_context;
239-
Context::swap(task_context, scheduler_context);
240-
}
229+
{
230+
let (_, last_task_context, next_task_context) = self.get_contexts();
231+
let last_task_context = last_task_context.unwrap();
232+
let next_task_context = next_task_context.unwrap();
233+
Context::swap(last_task_context, next_task_context);
234+
}
241235

242-
priv fn swap_in_task_from_running_task(&mut self, running_task: &mut Task) {
243-
let running_task_context = &mut running_task.saved_context;
244-
let next_context = &self.current_task.get_ref().saved_context;
245-
Context::swap(running_task_context, next_context);
236+
// XXX: Should probably run cleanup jobs
246237
}
247238

248-
249239
// * Other stuff
250240

251241
fn in_task_context(&self) -> bool { self.current_task.is_some() }
@@ -270,20 +260,42 @@ pub impl Scheduler {
270260
}
271261
}
272262

273-
// XXX: Hack. This should return &'self mut but I don't know how to
274-
// make the borrowcheck happy
275-
fn task_from_last_cleanup_job(&mut self) -> &mut Task {
276-
assert!(!self.cleanup_jobs.is_empty());
277-
let last_job: &'self mut CleanupJob = &mut self.cleanup_jobs[0];
278-
let last_task: &'self Task = match last_job {
279-
&RescheduleTask(~ref task) => task,
280-
&RecycleTask(~ref task) => task,
281-
&GiveTask(~ref task, _) => task,
263+
/// Get mutable references to all the contexts that may be involved in a
264+
/// context switch.
265+
///
266+
/// Returns (the scheduler context, the optional context of the
267+
/// task in the cleanup list, the optional context of the task in
268+
/// the current task slot). When context switching to a task,
269+
/// callers should first arrange for that task to be located in the
270+
/// Scheduler's current_task slot and set up the
271+
/// post-context-switch cleanup job.
272+
fn get_contexts(&mut self) -> (&'self mut Context,
273+
Option<&'self mut Context>,
274+
Option<&'self mut Context>) {
275+
let last_task = if !self.cleanup_jobs.is_empty() {
276+
let last_job: &'self mut CleanupJob = &mut self.cleanup_jobs[0];
277+
let last_task: &'self Task = match last_job {
278+
&RescheduleTask(~ref task) => task,
279+
&RecycleTask(~ref task) => task,
280+
&GiveTask(~ref task, _) => task,
281+
};
282+
Some(last_task)
283+
} else {
284+
None
282285
};
283286
// XXX: Pattern matching mutable pointers above doesn't work
284287
// because borrowck thinks the three patterns are conflicting
285288
// borrows
286-
return unsafe { transmute::<&Task, &mut Task>(last_task) };
289+
let last_task = unsafe { transmute::<Option<&Task>, Option<&mut Task>>(last_task) };
290+
let last_task_context = match last_task {
291+
Some(ref t) => Some(&mut t.saved_context), None => None
292+
};
293+
let next_task_context = match self.current_task {
294+
Some(ref mut t) => Some(&mut t.saved_context), None => None
295+
};
296+
return (&mut self.saved_context,
297+
last_task_context,
298+
next_task_context);
287299
}
288300
}
289301

@@ -313,6 +325,9 @@ pub impl Task {
313325
priv fn build_start_wrapper(start: ~fn()) -> ~fn() {
314326
// XXX: The old code didn't have this extra allocation
315327
let wrapper: ~fn() = || {
328+
// XXX: Should probably run scheduler cleanup jobs for situations
329+
// where a task context switches directly to a new task
330+
316331
start();
317332

318333
let mut sched = ThreadLocalScheduler::new();

0 commit comments

Comments
 (0)