search

launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

Invalid monkey patching of global function setImmediate #206

Closed esatterwhite closed 3 years ago

esatterwhite commented 3 years ago

The node sdk can potentially monkey patch setImmediate for all other code in the same process. These two functions have different behaviors and swapping setimmediate for nextTick can result in even loop starvation.

This behavior should be removed. If there is some kind of fallback logic needed here it should be encapsulated in a discrete function and that should be used instead.

eli-darkly commented 3 years ago

Thanks for catching this. It was left over from an extremely early version of the SDK written at a time when setImmediate was not guaranteed to be available in Node— although, as you say, monkey-patching was not an appropriate solution for this. Since setImmediate is available in every version of Node that we support now and will therefore never be null, this line is always a no-op (that is, unless some other code has chosen for some reason to monkey-patch the function out of existence with global.setImmediate = null, which would be likely to break all kinds of other code)— so the situation you're concerned about won't happen. But we should still remove the line.

eli-darkly commented 3 years ago

Removed this line in the 5.14.1 release.