Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix(task): fix #778, sometimes task will run after being canceled - #780

Merged
mhevery merged 1 commit into
angular:masterfrom
JiaLiPassion:task
May 19, 2017
Merged

fix(task): fix #778, sometimes task will run after being canceled#780
mhevery merged 1 commit into
angular:masterfrom
JiaLiPassion:task

Conversation

@JiaLiPassion

@JiaLiPassionJiaLiPassion commented May 15, 2017

Copy link
Copy Markdown
Collaborator

fix#778.

In normal case, ZoneTask can only transit to running state from scheduled state.
But in some complex cases, ZoneTask will try to run after being canceled.

varcreateInterface=require('readline').createInterface;varObservable=require('rx').Observable;require('zone.js');varrl=createInterface({terminal: true,input: process.stdin,output: process.stdout});varkeypress=Observable.fromEvent(rl.input,'keypress');varline=Observable.fromEvent(rl,'line');keypress.takeUntil(line).toPromise();

The case above describe such issue.

Be simplified, a target have 2 listeners for 1 event, and when the event trigger, the 1st event listener remove the 2nd, and then 2nd listener run will cause this issue.

The detailed process is:

  1. createInterface and Observable.fromEvent('keypress') will create two eventListeners of keypress event on ReadableStream target.
  2. keypress (fromEvent) also subscribe line Observable.
  3. when user enter key in command line.
    • keypress eventlisteners (2 listeners) will begin to run.
    • listeners[0] createInterface listener https://github.com/nodejs/node/blob/master/lib/readline.js#L187 will run normally, and trigger line Observable
    • line Observable will then trigger keypress Observable
    • keypress Observable will auto dispose and being removed from ReadableStream, so the Task is canceled
    • now ReadableStream only have one listener, but in memory listeners from the beginning still have 2
    • listeners[1] begin to run, but it has been canceled, and error occurs.

It is a little tricky, I tried without zone.js, the removed listener will also run again but do nothing (in this case, the observable has been stopped).

So in this PR, I just check whether ZoneTask is notScheduled (canceled) or not before runTask, if it is the case, just return and do nothing.

@mhevery
mhevery merged commit b7238c8 into angular:masterMay 19, 2017
@JiaLiPassion
JiaLiPassion deleted the task branch July 13, 2017 04:29
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eventTask Error with rx Observable

3 participants

@JiaLiPassion@mhevery@googlebot