Skip to content

Commit

Permalink
Don't crash when promise task is cancelled before its resolved (#29969)
Browse files Browse the repository at this point in the history
Summary:
This pull request fixes a potential `TypeError` in TaskQueue.js, that happens if a promise is added to the task queue, which is cancelled between the promise starting and resolving.

The exact error this resolves is
```js
TypeError: TaskQueue: Error resolving Promise in task gen1: Cannot set property ‘popable’ of undefined
      167 |           queueStackSize: this._queueStack.length,
      168 |         });
    > 169 |       this._queueStack[stackIdx].popable = true;
          |                                              ^
      170 |       this.hasTasksToProcess() && this._onMoreTasks();
      171 |     })
      172 |     .catch(ex => {
      at Libraries/Interaction/TaskQueue.js:169:46
```

This specific error was also reported in #16321

## Changelog

[General] [Fixed] - Prevent TypeError in TaskQueue when cancelling a started but not resolved promise.

Pull Request resolved: #29969

Test Plan:
The added test demonstrates the error, if run without the fixed applied to TaskQueue.js.
This is a race condition error, so is difficult to replicate!

Reviewed By: yungsters

Differential Revision: D23785972

Pulled By: appden

fbshipit-source-id: ddb8d06b37d296ee934ff39815cf5c9026d73871
  • Loading branch information
robwalkerco authored and facebook-github-bot committed Sep 26, 2020
1 parent 1438543 commit 14042fb
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
3 changes: 2 additions & 1 deletion Libraries/Interaction/TaskQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ class TaskQueue {
// happens once it is fully processed.
this._queueStack.push({tasks: [], popable: false});
const stackIdx = this._queueStack.length - 1;
const stackItem = this._queueStack[stackIdx];
DEBUG && infoLog('TaskQueue: push new queue: ', {stackIdx});
DEBUG && infoLog('TaskQueue: exec gen task ' + task.name);
task
Expand All @@ -166,7 +167,7 @@ class TaskQueue {
stackIdx,
queueStackSize: this._queueStack.length,
});
this._queueStack[stackIdx].popable = true;
stackItem.popable = true;
this.hasTasksToProcess() && this._onMoreTasks();
})
.catch(ex => {
Expand Down
16 changes: 16 additions & 0 deletions Libraries/Interaction/__tests__/TaskQueue-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,20 @@ describe('TaskQueue', () => {
expect(task1).not.toBeCalled();
expect(taskQueue.hasTasksToProcess()).toBe(false);
});

it('should not crash when task is cancelled between being started and resolved', () => {
const task1 = jest.fn(() => {
return new Promise(resolve => {
setTimeout(() => {
resolve();
}, 1);
});
});

taskQueue.enqueue({gen: task1, name: 'gen1'});
taskQueue.processNext();
taskQueue.cancelTasks([task1]);

jest.runAllTimers();
});
});

0 comments on commit 14042fb

Please sign in to comment.