Make function calls in for loop synchronous

Question

I get an array of strings in the request. Each string contains a command to be executed on the native shell.

var process = require('child_process'); 

function execCommand(req,res,callback){
 var params = req.params.list              //list is an array in the   request
 var result = '';
 var command = '';  
 for (var i = 0; i < params.length; i++) {
  command = params[i];
  cmd = process.exec(command);                        
  cmd.stdout.on('data', function(data){                       
   //push the shell output in variable result
  });
  cmd.on('close', function(code) {                            
   //convert the variable result to a valid JSON
  });
 }
 res.send(result);
};

The result of all the commands get mixed up in the result variable. How to make the function calls in the for loop synchronous?

Try to use library called `async`: https://github.com/caolan/async — vanadium23, May 06 '15 at 09:26
Possible duplicate of [Asynchronous Process inside a javascript for loop](https://stackoverflow.com/questions/11488014/asynchronous-process-inside-a-javascript-for-loop) — Liam, May 08 '18 at 12:36

Christopher Reid · Answer 1 · 2015-05-06T10:45:01.583

Use execSync instead of exec!

 for (var i = 0; i < params.length; i++) {
    command = params[i];
    result += process.execSync(command).toString();                        
 }

As others have pointed out, this might not be a very good idea as it will block the event loop and any requests that are being made. It is acceptable if the processes being executed will take a very short period of time to finish, but not otherwise.

Here is a simple pattern to control flow of asynchronous functions in a for loop... every time a process returns, params_finished increments by one. Once the number of finished processes equals the total number of processes the response is sent.

 for (var i = 0, total_params=params.length, params_finished=0; i < total_params; i++) {
    command = params[i];
    cmd = process.exec(command);                        
    cmd.stdout.on('data', function(data){                       
        //push the shell output in variable result
    });
    cmd.on('close', function(code) {                            
        //convert the variable result to a valid JSON
        params_finished++
        if(params_finished == total_params){
            res.send(result);
        }
    });
 }

It's a very bad idea to use synchronous methods in http server, because it will block `node.js` process. — Leonid Beschastny, May 06 '15 at 09:50
This a not a good approach. Don't use synchronous code if not really really needed. — Exinferis, May 06 '15 at 09:57

score 2 · Accepted Answer · answered May 06 '15 at 09:32

2

You can use execSync or...

You need to add a control flow library to help with asynchronous calls, so you can choose to run your functions in series or parallel:

Async: https://www.npmjs.com/package/async
Q (using promises, with Q.nfcall) https://www.npmjs.com/package/q

answered May 06 '15 at 09:32

Thomas Roch

1,159
7
17

This is most likely the best approach. – Exinferis May 06 '15 at 09:57

score 0 · Answer 3 · answered Jul 21 '17 at 11:58

An alternative would be to use the equivalent to a sleep function inside the loop. If you know more or less how long it takes for each instance of the loop.

function sleep(ms) {
      return new Promise(resolve => setTimeout(resolve, ms));
    }

async function myLoopFunction(){
    // any code before loop
    for (var i = 0; i < params.length; i++) {
       // do whatever
       await sleep(3000); // 3000 milisecond, for instance
    }
};

See this answer for a complete explanation about how this approach works.

score 0 · Answer 4 · answered Dec 12 '17 at 05:55

Use this function for a in serie synchronous for loop

function eachSeries(array, fn) {
    return new Promise(function (resolveGlobal, rejectGlobal) {

        var promises = []
        var next = 0

        fn(array[next], resolveObj, rejectObj)

        function resolveObj(data) {

            promises.push( Promise.resolve(data) )

            next++

            if (next >= array.length) {
                Promise.all(promises).then(function (data) {
                    resolveGlobal(data)
                }).catch(function (error) {
                    rejectGlobal(error)
                })
            } else {
                fn(array[next], resolveObj, rejectObj)
            }
        }

        function rejectObj(error) {
            return rejectGlobal(error)
        }
    })
}

And you use that function like this...

var process = require('child_process');

function execCommand(req,res,callback){
    var params = req.params.list              //list is an array in the   request
    var result = []

    eachSeries(params, function (param, resolve, reject) {
        cmd = process.exec(param)

        cmd.stdout.on('data', function (data){
            //... after long time you get data, then
            result.push(data.toObject())
            resolve()
        })

        cmd.on("error", function (error){
            reject(error) //If something bad happend
        })
    })
    .then(function () {
        console.log("All process executed one by one and pushed...")
        res.send(result)
    })
    .catch(function (error) {
        console.log("SOmething broke", error)
        res.send(error)
    })
}

or if you prefer

var process = require('child_process');

function execCommand(req,res,callback){
    var params = req.params.list              //list is an array in the   request

    eachSeries(params, function (param, resolve, reject) {
        cmd = process.exec(param)

        cmd.stdout.on('data', function (data){
            //... after long time you get data, then
            resolve(data.toObject())
        })

        cmd.on("error", function (error){
            reject(error) //If something bad happend
        })
    })
    .then(function (alldata) {
        res.send(alldata)
    })
    .catch(function (error) {
        res.send(error)
    })
}

Make function calls in for loop synchronous

4 Answers4

Linked