Replace recursive process.nextTick usage with setImmediate #11

Merged
merged 2 commits into from Aug 20, 2013

Projects

None yet

2 participants

@JulesAU
JulesAU commented Aug 20, 2013

As per http://blog.nodejs.org/2013/03/11/node-v0-10-0-stable/

You've got a recursive process.nextTick happening in gearman.js - and we're seeing this crash node occasionally with:

 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
 RangeError: Maximum call stack size exceeded

More info here:
http://blog.peakji.com/node-js-process-nexttick-and-setimmediate/

BTW it would be great to see these changes pushed to npm!

@andris9
Owner
andris9 commented Aug 20, 2013

setImmediate is not available in older versions of node. If used, there should be some kind of fallback option, eg. if setImmediate support is not detected, process.nextTick is used instead

@JulesAU
JulesAU commented Aug 20, 2013

Good point - have now added a fallback mechanism.

@andris9 andris9 merged commit af9d8dd into andris9:master Aug 20, 2013

1 check failed

Details default The Travis CI build failed
@JulesAU
JulesAU commented Aug 20, 2013

Great - thanks. Any chance we could get the version bumped and pushed to npm? Seems to be rather out of date over there: https://npmjs.org/package/node-gearman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment