Skip to content

Commit

Permalink
Ensure actor cancel callbacks are cleaned up (#13074)
Browse files Browse the repository at this point in the history
* Ensure cancel callbacks are cleaned up

* Remove duplicate delete call

---------

Co-authored-by: Stepan Kuzmin <[email protected]>
  • Loading branch information
temas and stepankuzmin authored Feb 13, 2024
1 parent 6014d7d commit bb51e3b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
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];
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

0 comments on commit bb51e3b

Please sign in to comment.