tomas / wmic

Wrapper around the Windows WMIC interface for Node.js.
12 stars 15 forks source link

replace process.nextTick() by setImmediate() #10

Closed epieddy closed 3 years ago

epieddy commented 4 years ago

Hi,

I've replaced you usage of process.nextTick() by setImmediate().

There is a real difference between the two : https://nodejs.org/en/docs/guides/event-loop-timers-and-nexttick/#process-nexttick-vs-setimmediate

basically process.nextTick() will fire the callback immediatly after the callstack is empty when setImmediate() will give a chance to nodejs to poll some IO before calling the callback().

And that is exactly the bug I've had :

On some occasion, I've received the 'exit' event before the 'data' event on stdout. With process.nextTick(), cb(null, [stdout, stderr]) is called before IO are pulled and the output of the command is simply ignored.

By using setImmediate(), even if the 'exit' event is fired first, nodejs will have a chance to poll some IO and fire the 'data' event after the 'exit' event BUT BEFORE cb(null, [stdout, stderr]) is called.

Can you please merge this quickly please.

Regards

epieddy commented 4 years ago

I've had the bug with

WMIC.get_values('logicaldisk', 'name,size,freespace', null, (error, list: any[]) => {

// list is empty because no output has been seen and parsed
});