Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure actor cancel callbacks are cleaned up #13074

Merged
merged 4 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/util/actor.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ class Actor {
// We're using a MessageChannel object to get throttle the process() flow to one at a time.
const callback = this.callbacks[id];
const metadata = (callback && callback.metadata) || {type: "message"};
this.cancelCallbacks[id] = this.scheduler.add(() => this.processTask(id, data), metadata);
const cancel = this.scheduler.add(() => this.processTask(id, data), metadata);
if (cancel) this.cancelCallbacks[id] = cancel;
} else {
// In the main thread, process messages immediately so that other work does not slip in
// between getting partial data back from workers.
Expand All @@ -123,6 +124,8 @@ class Actor {
}

processTask(id: number, task: any) {
// Always delete since we are no longer cancellable
delete this.cancelCallbacks[id];
temas marked this conversation as resolved.
Show resolved Hide resolved
if (task.type === '<response>') {
// The done() function in the counterpart has been called, and we are now
// firing the callback in the originating actor, if there is one.
Expand All @@ -139,7 +142,6 @@ class Actor {
} else {
const buffers: Set<Transferable> = new Set();
const done = task.hasCallback ? (err: ?Error, data: mixed) => {
delete this.cancelCallbacks[id];
this.target.postMessage({
id,
type: '<response>',
Expand Down
7 changes: 3 additions & 4 deletions src/util/scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Scheduler {
this.nextId = 0;
}

add(fn: TaskFunction, metadata: TaskMetadata): Cancelable {
add(fn: TaskFunction, metadata: TaskMetadata): Cancelable | null {
const id = this.nextId++;
const priority = getPriority(metadata);

Expand All @@ -50,9 +50,8 @@ class Scheduler {
} finally {
if (m) PerformanceUtils.endMeasure(m);
}
return {
cancel: () => {}
};
// Don't return an empty cancel because we can't actually be cancelled
return null;
}

this.tasks[id] = {fn, metadata, priority, id};
Expand Down
Loading