Skip to content

Refactor ProcessEdgesBase::worker to make it safe #1040

Open
@wks

Description

@wks

The ProcessEdgesBase::worker field is a raw pointer *mut GCWorker<VM>. It is initialised as null when instantiating ProcessEdgesBase, but later set to the pointer to the worker in do_work(). It is used in

  • ProcessEdgesWork::trace_object: It will dispatch to the trace_object method of different spaces. But since trace_object can copy object, it needs its copy_context to do the copying. For this reason, we need to pass the worker (or the underlying CopyContext to the trace_object method of concrete spaces.
  • ProcessEdgesWork::start_or_dispatch_scan_work: If it decides to execute the ScanObjects work packet immediately after tracing all slots (Edge), it will call ScanObjects::do_work, and one parameter is the GC worker.

Obviously, raw pointers are unsafe.

The root problem is that the ProcessEdgesWork work packet is instantiated when enqueuing the slots, but the reference to the GCWorker is only available when executing the ProcessEdgesWork work packet.

There are two obvious solutions:

  1. Pass the &mut GCWorker from do_work() all the way through process_edges(), process_edge() and trace_object(). But also be ware that ProcessEdgesWork::trace_object is sometimes called directly, bypassing process_edges. For example,
fn do_work(&mut self, worker: &mut GCWorker, mmtk: &MMTK) {
    self.process_edges(worker);
}
fn process_edges(&mut self, worker: &mut GCWorker) {
    for edge in self.edges {
        self.process_edge(edge, worker);
    }
}
fn process_edge(&mut self, edge: Edge, worker: &mut GCWorker) {
    edge.store(self.trace_object(edge.load(), worker)));
}
fn trace_object(&mut self, object: ObjectReference, worker: &mut GCWorker) -> ObjectReference { ... }
  1. Create another struct that contains the slots (Edge) to process as well as a reference to GCWorker, and move the process_edges, process_edge and trace_object methods to that struct. Because the struct is constructed when a &mut GCWorker is available, the worker field (as shown below) is safe, but has a lifetime annotation. For example,
struct DoProcessEdges<'w> { // This struct has a lifetime param `'w` which means it only lives when a reference to the worker is available.
    edges: Vec<Edge>,
    worker: &'w mut GCWorker<VM>, // Borrows the reference of the worker during its lifetime.
    // other fields here.
}

impl<'w> DoProcessEdges<'w> {
    fn process_edges(&mut self) { ... }
    fn process_edge(&mut self, edge: Edge) { ... }
    fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { ... }
}

Related issues and PRs

There is a stale PR for solving the problem by adding the parameter worker: &mut GCWorker to several functions. #597

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-work-queueArea: Work queueC-refactoringCategory: RefactoringF-projectCall For Participation: Student projects. These issues are available student projects.G-safetyGoal: SafetyP-normalPriority: Normal.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions