child_process: fork does not accept null or undefined args array #10819

Activity

1. joyeecheung

   added

   child_processIssues and PRs related to the child_process subsystem.Issues and PRs related to the child_process subsystem.

   on Jan 15, 2017
2. Fishrock123 commented on Jan 16, 2017

   

   Fishrock123

   on Jan 16, 2017

   Contributor

   More actions

   I think it would make more sense to throw a TypeError over seeing it as no-args? What does everyone else think?
3. sam-github commented on Jan 16, 2017

   

   sam-github

   on Jan 16, 2017

   Contributor

   More actions

   The handling should be the same across the child_process APIs for when argv arrays are passed null or undefined. What do execFile/spawn[Sync]() do in this case?
4. trendsetter37 commented on Jan 16, 2017

   

   trendsetter37

   on Jan 16, 2017

   Last edited by trendsetter37

   ContributorAuthor

   More actions

   @sam-github spawn and execFile uses a normalizeSpawnArguments function to parse arguments.

   var spawn = exports.spawn = function(/*file, args, options*/) {
     var opts = normalizeSpawnArguments.apply(null, arguments);
     var options = opts.options;
     var child = new ChildProcess();
   
     debug('spawn', opts.args, options);
   
     child.spawn({
       file: opts.file,
       args: opts.args,
       cwd: options.cwd,
       windowsVerbatimArguments: !!options.windowsVerbatimArguments,
       detached: !!options.detached,
       envPairs: opts.envPairs,
       stdio: options.stdio,
       uid: options.uid,
       gid: options.gid
     });

   where as fork does something different...or it's own thing entirely rather.
5. sam-github commented on Jan 16, 2017

   

   sam-github

   on Jan 16, 2017

   Contributor

   More actions

   How those functions are implemented internally is their own business, how they behave should be consistent. spawn and exec didn't used to share implementation, and their behaviour was bafflingly inconsistent, the code sharing helps ensure they behave identically, as do the tests.
6. Trott commented on Jul 16, 2017

   

   Trott

   on Jul 16, 2017

   Member

   More actions

   @sam-github Should this remain open?
7. sam-github commented on Jul 21, 2017

   

   sam-github

   on Jul 21, 2017

   Contributor

   More actions

   Yes, it sounds like there is an inconsistency, but no one has looked into it yet.
8. gireeshpunathil commented on May 14, 2018

   

   gireeshpunathil

   on May 14, 2018

   Member

   More actions

   this is addressed through #10866 and is a close candidate? I may be wrong.
9. laykun commented on May 29, 2018

   

   laykun

   on May 29, 2018

   More actions

   I wasted a whole bunch of time figuring out why my child processes were crashing due to invalid debug options even when I was passing in a new inspect port to use. Turns on I was using null as my args argument assuming it would NOT ignore the third argument in the fork function. This is very odd behaviour and will cost more time. The function should process arguments on a case by case basis and not be dependent on prior arguments. Seeing as this behaviour is not mentioned in the documentation and using null / undefined as an argument should be perfectly acceptable then this behaviour needs to change.

   👍React with 👍1Reacted by Gireesh Punathil
10. gireeshpunathil commented on May 29, 2018

    

    gireeshpunathil

    on May 29, 2018

    Member

    More actions

    #20882 is a work in progress to address the ambiguity. Feel free to review that PR and provide any inputs, thanks!

    👍React with 👍1Reacted by Shobhit Chittora
11. juanarbol commented on Apr 26, 2020

    

    juanarbol

    on Apr 26, 2020

    Member

    More actions

    This was fixed on #22416

    Closing this issue, thanks for the report!
12. juanarbol

    closed this as completedon Apr 26, 2020