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

Infinite loop in timers.js #9756

Closed
hassy opened this issue Nov 23, 2016 · 0 comments
Closed

Infinite loop in timers.js #9756

hassy opened this issue Nov 23, 2016 · 0 comments
Labels
confirmed-bug Issues with confirmed bugs. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@hassy
Copy link
Contributor

hassy commented Nov 23, 2016

  • Version: 6.8.1 and later
  • Platform: tested on OSX & Linux, probably all platforms
  • Subsystem: timers

v6.8.1 and above will all hang when the following code is run:

var i1 = setImmediate(function() {
  console.log('i1 cb');
  clearImmediate(i2);
  clearImmediate(i3);
});

var i2 = setImmediate(function() {
  console.log('i2 cb');
});

var i3 = setImmediate(function() {
  console.log('i3 cb');
});

console.log('All set');

An example run of the code above:

hassy@mdb $ node -v
v4.3.2
hassy@mdb $ node isolated.js

All set
i1 cb
#
# The process exits immediately.
#
hassy@mdb  $ nvm use 6.8.1
hassy@mdb  $ node -v
v6.8.1
hassy@mdb $ node isolated.js

All set
i1 cb
#
# The process won't exit. top shows it using 98%+ CPU.
#

The cause is Node entering into an infinite loop here when immediate._onImmediate is null:
(source: https://github.com/nodejs/node/blob/master/lib/timers.js#L580-L587)

  while (immediate) {
    domain = immediate.domain;

    if (!immediate._onImmediate)
      continue;

This happens when the immediate referred to by next gets cleared.

The issue first manifests itself in this commit: 42158a0

The if branch that causes an infinite loop was first introduced in

node/lib/timers.js

Lines 514 to 519 in 6f75b66

while (L.isEmpty(queue) === false) {
immediate = L.shift(queue);
domain = immediate.domain;
if (!immediate._onImmediate)
continue;
but is obsolete now since the value of immediate will not change once the condition is hit.

I'll send a PR to go along with this with a proposed fix.

EDIT 1: Simplified the test case.

@mscdex mscdex added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Nov 23, 2016
hassy added a commit to artilleryio/arrivals that referenced this issue Dec 8, 2016
Stop the timer when the expected number of arrivals is reached rather
than with another timeout.

This is to avoid triggering a Node.js bug that leads to using
100% CPU.

Ref: nodejs/node#9756
@Fishrock123 Fishrock123 added the confirmed-bug Issues with confirmed bugs. label Dec 9, 2016
hassy added a commit to hassy/node that referenced this issue Dec 13, 2016
If current immediate has no callback, move on to the next one in
the queue.

Fixes: nodejs#9756
PR-URL: nodejs#9759
MylesBorins pushed a commit that referenced this issue Dec 15, 2016
If current immediate has no callback, move on to the next one in
the queue.

Fixes: #9756
PR-URL: #9759
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this issue Jan 22, 2017
If current immediate has no callback, move on to the next one in
the queue.

Fixes: #9756
PR-URL: #9759
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
If current immediate has no callback, move on to the next one in
the queue.

Fixes: #9756
PR-URL: #9759
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
If current immediate has no callback, move on to the next one in
the queue.

Fixes: #9756
PR-URL: #9759
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

No branches or pull requests

3 participants