9

My ex boss had a weird bug where when he used setInterval with a long delay interval:

setInterval(func, 3000000 /*50 minutes*/);

Node.js crashed.

func can be even a simple function that simply console.log('something').

Someone suggested him to wrap an anonymous function around func, and it actually solved his issue.

As much as I know, it shouldn't make a difference and even considered to be a bad practice at least in browsers' javascript.

Is there a difference in Node.js between

  • setInterval(func, delay)
  • setInterval(function(){func()}, delay)

or is it a bug in Node.js?

UPDATE:

Bug report on GitHub

Community
  • 1
  • 1
gdoron
  • 147,333
  • 58
  • 291
  • 367
  • I do not see why it would be considered bad practice to have an anonymous function instead of a function name... Curious what others have to say. I would consider the two examples to have identical outcome – mplungjan Jul 23 '15 at 20:01
  • @mplungjan of course it's bad practice in general JavaScript, it's wrapping a function for no reason and typically is an indication of someone not really understanding functions are first class in JS. – Benjamin Gruenbaum Jul 23 '15 at 20:03
  • 1
    I wonder won't `func` just be gcollected here because the reference in `setInterval` somehow doesn't count... Very good question. – raina77ow Jul 23 '15 at 20:05
  • @BenjaminGruenbaum Hmm, I see. I have never been in the situation so I guess I did not think it through. – mplungjan Jul 23 '15 at 20:05
  • The symptoms suggest that some time between when setInterval was called and 50 minutes passing the function becomes undefined. but why? – Kevin B Jul 23 '15 at 20:06
  • Sure, I know how to use both very well, I just did not see the major issue in calling a function inside an anonymous function since I never called just one function without parms inside such a function – mplungjan Jul 23 '15 at 20:34
  • 1
    Pretty sure that this is a bug in Node/io.js. – Benjamin Gruenbaum Jul 23 '15 at 20:42
  • I'd have to ask "does it always crash?" Fifty minutes is a very long time for a processor. A typical NTP polling interval might be a minute or so. If you can't recreate the bug when NTP is turned off this might be part of the problem. – Michael Blankenship Jul 24 '15 at 00:28

2 Answers2

1

To address your question directly: Yes, there is a difference. Not in the functionality of setInterval, but in the scope the function will have when it's run.

Scenario 1:

setInterval(function() {
  console.trace();
}, 3000000);

Every 50 minutes, a stack trace will be printed to console. In this case because the function console.trace has been called directly it will maintain the context of console.

Scenario 2:

setInterval(console.trace, 3000000);

This will throw an error because it will be invoked with the context of the scope that executes it, not console. To maintain context, you can pass a bound copy of the function:

setInterval(console.trace.bind(console), 3000000);

It doesn't seem the problem your boss had can be reproduced today. So, as others have suggested, it may be a problem with garbage collection. However, I would be more inclined to believe that the function your boss was calling depended on a specific context that was maintained via anonymous function but lost when passing an unbound function.

rrowland
  • 2,734
  • 2
  • 17
  • 32
0

It's possible that V8 is doing garbage collection behind-the-scenes by removing whatever function you're referencing here as "func". This would leave "func" pointing to an invalid function.

Try running node with the following argument:

node --expose-gc app.js

...and then in your code after the setInterval(func, 10000) call:

global.gc();

...which will trigger the garbage collection in V8. Since I've dropped the timer down to ten seconds you can test the theory without waiting an entire hour.

It's entirely possible that wrapping an anonymous function call then prevents garbage collection from removing your actual function from memory.

Note that you can also start with:

 node --trace-gc app.js

...which will log garbage collection by V8. It might tell you something interesting. See https://groups.google.com/forum/#!topic/nodejs/gITsLR7Zkew

Michael Blankenship
  • 1,639
  • 10
  • 16
  • I don't see why this would happen. There is still a reference to `createSasTokenTimer`. When `setInterval` is called, it creates a `timer` instance which has a property called `_onTimeout`. An inner function called `wrapper` is assigned to `_onTimeout`, and `wrapper` itself calls the `callback`, which is the callback passed into `setInterval`, which is ultimately `createSasTokenTimer`. Even if the main context is disposed, there is still a reference and so the callback is still "alive". It also doesn't explain why the anonymous function doesn't end up getting GC'd. – Vivin Paliath Jul 24 '15 at 21:18
  • I would guess here that if you wrap it in an anonymous call it's then going a different path, for example, it's in the threadpool instead of being run in the main thread. – Michael Blankenship Jul 24 '15 at 22:24
  • There is only one thread in node. But that's besides the point because it doesn't have anything to do with threading. The OP is wondering why the callback ends up being undefined after a while when it is passed in as a reference to a function declaration, vs. being passed in as a reference to an anonymous function. From the perspective of `setInterval`, there is no difference since it gets a reference to a function in both cases. – Vivin Paliath Jul 24 '15 at 22:29
  • http://stackoverflow.com/questions/10680601/nodejs-event-loop Per someone knowledgeable: "node.js has 1 permanent event loop: that's the v8 event loop. To handle C++ async tasks it's using a threadpool [via libeio & libev ]." – Michael Blankenship Jul 24 '15 at 22:48
  • I was talking about the event loop; not about the workers. That being said, *how* you pass in a callback (be it a reference to a declared function or a function expression) should have nothing to do with it being executed in the main thread or through the thread-pool. – Vivin Paliath Jul 24 '15 at 22:57
  • I run @MichaelBlankenship code, and the app did not crash. – janiv Jul 26 '15 at 08:46
  • @janiv Okay, if you ran global.gc() and the call to func() fired off after this and it *didn't* crash then you ought to be able to rule out V8's garbage collection as the cause. I think you're left with a scope-related issue now. – Michael Blankenship Jul 28 '15 at 19:39