Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Debugger does not respect timer #9333

Closed
hankduan opened this issue Mar 5, 2015 · 10 comments
Closed

Debugger does not respect timer #9333

hankduan opened this issue Mar 5, 2015 · 10 comments

Comments

@hankduan
Copy link

hankduan commented Mar 5, 2015

This was originally #9319, but I've reduced the case down more.
Note: this is happening with node 0.10.31 and above.

Given code example.js:

setInterval(function() {  
  console.log(1)
  debugger
  console.log(2);
}, 10);

ok I enter into debugger now:

$ node debug ./example.js 
< Debugger listening on port 5858
connecting to port 5858... ok
break in example.js:1
> 1 setInterval(function() {  
  2   console.log(1)
  3   debugger
debug> c
< 1
break in example.js:3
  1 setInterval(function() {  
  2   console.log(1)
> 3   debugger
  4   console.log(2);
  5 }, 10);
debug> c    //*********now wait 1min before you hit enter******//
< 2 //********now it gets stuck here for 1min before it moves onto the next break point****//

My assumption is that the break points are messing up node's timer some how.

This may seem like a small issue in the debugger, but it's causing huge pains for Protractor users because all of protractor's code (which is using webdriver) is in a giant setInterval.

reference:
angular/protractor#1890
angular/protractor#1899
angular/protractor#1822
angular/protractor#1890
angular/protractor#1893

and the only workaround for that right now is to downgrade node to 0.10.30 or lower.

Thanks!

@hankduan
Copy link
Author

hankduan commented Mar 5, 2015

fyi: the break is because of this change: 6f04394#diff-a7a2d51d79b1c80c7458e0d056e3d86fR167

i.e. if I remove this line (https://github.com/joyent/node/blob/v0.12.0/src/timer_wrap.cc#L150) it is fixed. However, I don't know enough about node to make the fix without breaking that commit's intentions unfortunately =(

@jperry
Copy link

jperry commented Mar 5, 2015

please fix asap as this is a painful bug.

@misterdjules
Copy link

Closing as duplicate of #9125, but the information in this issue may be very helpful to investigate it, so thank you very much @hankduan! 👍

@misterdjules
Copy link

Actually, after investigating this more carefully, it's not clear to me it's a duplicate of #9125. Reopening.

@misterdjules
Copy link

It seems it also happens on v0.12.

@misterdjules
Copy link

This is the same issue as #15447. The diff proposed there, although it will not necessarily be committed as is, fixes this issue.

misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 21, 2015
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes nodejs#9333 and nodejs#15447.
@misterdjules misterdjules self-assigned this Apr 21, 2015
@misterdjules
Copy link

Created a PR that fixes the issue: #17203. Thank you very much @hankduan!

@hankduan
Copy link
Author

Thanks a bunch!

misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 21, 2015
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes nodejs#9333 and nodejs#15447.
misterdjules pushed a commit to misterdjules/node that referenced this issue Jun 17, 2015
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes nodejs#9333 and nodejs#15447.
misterdjules pushed a commit to misterdjules/node that referenced this issue Jun 17, 2015
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes nodejs#9333 and nodejs#15447.

PR: nodejs#17203
PR-URL: nodejs#17203
Reviewed-By: Fedor Indutny <[email protected]>
@misterdjules
Copy link

Fixed by d38e865, which will be available in the upcoming v0.10.39 and v0.12.5 releases.

@hankduan
Copy link
Author

Thanks @misterdjules!

Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Jul 24, 2015
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447

PR: nodejs/node-v0.x-archive#17203
PR-URL: nodejs/node-v0.x-archive#17203
Reviewed-By: Fedor Indutny <[email protected]>

Conflicts:
	lib/timers.js
	test/common.js
whitlockjc added a commit to whitlockjc/node that referenced this issue Jul 15, 2016
Whenever a timer is scheduled within another timer, there are a few
known issues that we are fixing:

* Whenever the timer being scheduled has the same timeout value as the
outer timer, the newly created timer can fire on the same tick of the
event loop instead of during the next tick of the event loop
* Whenever a timer is added in another timer's callback, its underlying
timer handle will be started with a timeout that is actually incorrect

This commit consists of
nodejs/node-v0.x-archive#17203 and
nodejs/node-v0.x-archive#25763.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447
Fixes: nodejs/node-v0.x-archive#25607
Fixes: nodejs#5426
PR-URL: nodejs#3063
whitlockjc added a commit to nodejs/node that referenced this issue Jul 15, 2016
Whenever a timer is scheduled within another timer, there are a few
known issues that we are fixing:

* Whenever the timer being scheduled has the same timeout value as the
outer timer, the newly created timer can fire on the same tick of the
event loop instead of during the next tick of the event loop
* Whenever a timer is added in another timer's callback, its underlying
timer handle will be started with a timeout that is actually incorrect

This commit consists of
nodejs/node-v0.x-archive#17203 and
nodejs/node-v0.x-archive#25763.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447
Fixes: nodejs/node-v0.x-archive#25607
Fixes: #5426
PR-URL: #3063
evanlucas pushed a commit to nodejs/node that referenced this issue Jul 19, 2016
Whenever a timer is scheduled within another timer, there are a few
known issues that we are fixing:

* Whenever the timer being scheduled has the same timeout value as the
outer timer, the newly created timer can fire on the same tick of the
event loop instead of during the next tick of the event loop
* Whenever a timer is added in another timer's callback, its underlying
timer handle will be started with a timeout that is actually incorrect

This commit consists of
nodejs/node-v0.x-archive#17203 and
nodejs/node-v0.x-archive#25763.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447
Fixes: nodejs/node-v0.x-archive#25607
Fixes: #5426
PR-URL: #3063
evanlucas pushed a commit to nodejs/node that referenced this issue Jul 20, 2016
Whenever a timer is scheduled within another timer, there are a few
known issues that we are fixing:

* Whenever the timer being scheduled has the same timeout value as the
outer timer, the newly created timer can fire on the same tick of the
event loop instead of during the next tick of the event loop
* Whenever a timer is added in another timer's callback, its underlying
timer handle will be started with a timeout that is actually incorrect

This commit consists of
nodejs/node-v0.x-archive#17203 and
nodejs/node-v0.x-archive#25763.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447
Fixes: nodejs/node-v0.x-archive#25607
Fixes: #5426
PR-URL: #3063
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
When a timer is added in another timer's callback, its underlying timer
handle will be started with a timeout that is actually incorrect.

The reason is that  the value that represents the current time is not
updated between the time the original callback is called and the time
the added timer is processed by timers.listOnTimeout. That leads the
logic in timers.listOnTimeout to do an incorrect computation that makes
the added timer fire with a timeout of scheduledTimeout +
timeSpentInCallback.

This change fixes that and make timers scheduled within other timers'
callbacks fire as expected.

Fixes nodejs#9333 and nodejs#15447.

PR: nodejs#17203
PR-URL: nodejs#17203
Reviewed-By: Fedor Indutny <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants