spawn sync only with --fork #25

pibi · 2016-05-09T19:58:44Z

Mongod doesn't support --fork on Windows. so forcing it to true leads to errors (see mccormicka/Mockgoose#207 ). Without --fork spawnSync will block the event loop indefinitely.

So, here is a fix for both the issues.

seriousManual · 2016-05-10T08:26:58Z

I'm also getting the error message "Error parsing command line: unrecognised option '--fork'" (windows), when I used the fix from above it worked again.

winfinit · 2016-05-10T14:01:55Z

@pibi, there are two reasons why fork was used, one is consistent reporting of child process status, so I didn't have to do bunch of catches, and second reason is to deprecate event emitter. I would like to see if there is a way to write a wrapper around mongod for windows, that would simulate interface of linux/osx, for example, it could be a "start /b" check on a child process, and then report back status that can be parsed by mongodb-prebuilt

pibi · 2016-05-10T23:13:13Z

Ok, I can understand the event emitter deprecation reason. Maybe I don't understand the former reason because I'm using your project inside mockgoose to perform unit tests. For this use case '--fork' (daemon mode) is pretty useless. Well, I don't know a use case where I can use mongodb-prebuilt without a proper detach of mongod.

winfinit · 2016-05-10T23:45:58Z

It will detach, and spawn killer process that will take care of a mongod, otherwise in case of parent death, mongod would remain alive. Your unit tests should not care nor know about mongod spawning of with or without fork.

On May 10, 2016, at 7:13 PM, pibi notifications@github.com wrote:

Ok, I can understand the event emitter deprecation reason. Maybe I don't understand the former reason because I'm using your project inside mockgoose to perform unit tests. For this use case '--fork' (daemon mode) is pretty useless. Well, I don't know a use case where I can use mongodb-prebuilt without a proper detach of mongod.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub

pibi · 2016-05-11T07:59:31Z

It will detach, and spawn killer process that will take care of a mongod, otherwise in case of parent death, mongod would remain alive.

This makes no sense for me. You should use spawnSync() to get an output for a short-living process, while you should use spawn() exactly to have control over the child process. Anyway, both cases should not leave any mongod instance alive because this is how child_process works: it spawn process under the same process tree, so, if parent died, then OS will kill all the children.
mongokiller.js is quite useless, but I can understand it if you use spawn() with option.detach ( https://nodejs.org/api/child_process.html#child_process_options_detached).

It seems that --fork is used as a shortcut for child process spawning delegation to an external process, you save the bunch of event emitter code, but at the price of less control over the mongod process itself, less portability and the need of an external watchdog (mongokiller).

nodejs child_process is a portable module, you should stick to it for process management. 'start /b' is again another shortcut that gives you additional work, while you can use options.detach on spawn

Your unit tests should not care nor know about mongod spawning of with or without fork.

yep, I agree. I was talking about the usage of this module. I have no visibility about its usage in different contexts, so from a "test point of view" --fork is useless.

winfinit · 2016-05-11T12:37:37Z

@pibi there is somewhere a disconnect, and I am not sure where. mongod is not a short lived process, and if you would use spawnSync (which I am using) on mongod without --fork, node process will be blocked until command is not done executing. Now if you use just spawn, you have two issues, one is that you have to attach to output, and start parsing output from mongod to determine if service started successfully or not, and exit codes across platforms and mongod versions are not consistent, so you cannot rely on exit codes, we tried that already, if you go with parsing logs, that means you have to notify "something" (in this case Mockgoose) that database is started (or in failure case you have to notify mongodb-prebuilt). What fork does in mongod is: it start a process on the background, but it reports startup status and exit codes in a synchronous matter, i.e.: process doesnt exit until mongod is initiated.

reason why you cannot just spawn, and go on with your life, is because you can get race condition even in a successful scenario, and have to implement retries on a client side: i.e.: if you spawned mongod, and it took longer to start, before connect call is being issued. Now in this retry scenario, it is not working also, in case there was a failure to start mongod, because client then would be retrying infinitely, unless there is a process to detect a failure of a startup, and try different port for example.

now detached vs not detached, will not help you, if process simply dies, child process remains, for this you can setup a simple test where you spawnSync process (with or without detach), and kill -9 your parent process, then check your process tree for mongod.

pibi · 2016-05-11T14:35:02Z

@winfinit first of all, thanks for sharing your thoughts with me.
About status code checking: ok, you are using --fork and exit status of the first mongod process to see if it is successfully started. I see also that you need to parse the outputs to get the child_process pid (to be able to kill it). Without --fork, you cannot check the exit status but you still have to parse outputs. pid of child is useless here, like mongokiller (see below).

Race condition: this happens because you are not using callback (see #27) . Client should not try to connect until a successful callback is fired (and this is the reason behind my patch at mccormicka/Mockgoose#212) .

About child process: As long as the parent exits or it is killed there's no way a child could survive, unless you use inherits/stream for stdio or detached. To be sure about that I just test these scenarios on windows/linux with this gist: https://gist.github.com/pibi/d6c90e23a9224164893cbe6882bc8b5c

winfinit · 2016-05-11T15:15:19Z

@pibi what are you demonstrating with above gists? issue is with killing a parent process, or if there is an exception that occurred within a parent process.

so either one of those cases would leave 'yes' dormant:

var spawn = require('child_process').spawn;
//var child = spawn('yes', { detached: true });
var child = spawn('yes');
require('breakme');

output:

Romans-MacBook-Air:bugs winfinit$ ps auxwww |grep yes
winfinit        19474   0.0  0.0  2434840    748 s001  R+   11:08AM   0:00.00 grep yes
Romans-MacBook-Air:bugs winfinit$ node a.js 
module.js:341
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:339:15)
    at Function.Module._load (module.js:290:25)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at Object.<anonymous> (/Users/winfinit/Documents/workspace/mongodb-prebuilt/bugs/a.js:6:1)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
Romans-MacBook-Air:bugs winfinit$ ps auxwww |grep yes
winfinit        19476  98.8  0.0  2434824    648 s001  R    11:08AM   0:03.13 yes
winfinit        19481   0.0  0.0  2434840    760 s001  S+   11:08AM   0:00.00 grep yes

now about callback, I need to look at it closer, from what i've read on previous bugs, issue is that you are dealing with some of the code that i've restructured, or didn't complete to restructure, that resulted in bugs due to insufficient testing.

one of those bugs is --fork and windows, which is separate problem from callback not being called, and all efforts of previous restructure were to block process until mongod started.

I will scan through reported issues in Mockgoose sometime this week, because at the moment I feel that all of the fixes that are being suggested are not aligned with my original rewrite, and people are fixing or reversing original vision, however I haven't been active for some time, so I can't blame anyone for this but me.

pibi · 2016-05-11T16:12:46Z

@winfinit yep, I know we are talking about many different issues here.

About the gist, yep, they are used to test the various scenario on win and linux. I just test your snippet: here the results:

Ubuntu, no detached:

me@ubuntu:/tmp/child_process# node yes.js 
module.js:340
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:289:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/tmp/child_process/yes.js:4:1)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)
    at Function.Module.runMain (module.js:467:10)
me@ubuntu:/tmp/child_process# pgrep yes
me@ubuntu:/tmp/child_process#

Ubuntu, with detached ( var child = spawn('yes', { detached: true , stdio:'ignore'}); ):

me@ubuntu:/tmp/child_process# node yes.js 
module.js:340
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:289:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/tmp/child_process/yes.js:4:1)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)
    at Function.Module.runMain (module.js:467:10)
me@ubuntu:/tmp/child_process# pgrep yes
3758
me@ubuntu:/tmp/child_process#

windows (mingw) with detach:

me@b121 MINGW64 /d
$ node yes.js
module.js:327
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (D:\yes.js:4:1)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)

me@b121 MINGW64 /d
$ ps -aef
     UID     PID    PPID  TTY        STIME COMMAND
   me    6876       1 ?        17:48:55 /usr/bin/yes
   me    6256    6524 cons0    17:48:58 /usr/bin/ps
   me    6524       1 cons0    17:42:24 /usr/bin/bash

windows (mingw) without detach:

me@b121 MINGW64 /d
$ node yes.js
module.js:327
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (D:\yes.js:4:1)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)

me@b121 MINGW64 /d
$ ps -aef
     UID     PID    PPID  TTY        STIME COMMAND
   me    4724    6524 cons0    17:51:23 /usr/bin/ps
   me    6524       1 cons0    17:42:24 /usr/bin/bash

windows, with detach (cmd and starting another long running node.js script) :

D:\>node yes
module.js:327
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (D:\yes.js:4:1)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)

D:\>tasklist /FI "IMAGENAME eq node.exe"

Nome immagine                  PID Nome sessione    Sessione n. Utilizzo mem
========================= ======== ================ =========== ============
node.exe                      6088 Console                    1     15.148 K

D:\> taskkill /F /IM node.exe

windows, without detach (cmd and starting another long running node.js script):

D:\>node yes
module.js:327
    throw err;
    ^

Error: Cannot find module 'breakme'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (D:\yes.js:4:1)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)

D:\>tasklist /FI "IMAGENAME eq node.exe"
Informazioni: nessuna attività in esecuzione corrispondente ai
criteri specificati.

D:\>

So, it seems you have a bug on your Mac!

pibi · 2016-05-11T16:24:07Z

maybe related to nodejs/node#3596

winfinit · 2016-05-11T16:34:58Z

@pibi not sure if comment meant to be witty, but it is not a bug on my mac, it is a behavior with OS X and/or BSD and nodeJS.

I don't want to dig around right now, but I do remember seeing various bugs filed against nodejs for this exact behavior, and personally was able to reproduce this exact thing on linux distros.

given that there is this behavior/bug, Mockgoose and mongodb-prebuilt would have to account for this, hence that is why i have a watchdog process, and would like to simulate --fork behavior on windows.

pibi · 2016-05-11T16:50:29Z

@winfinit: oh, ok, now I understand your reasons, it's a quite weird approach if you trust POSIX behavior to be consistent and cross-platform (here should be libuv to enforce the "standard").

About the --fork simulation, I'm wondering how it could be different from spawn(), either way you should parse the output log for error checking.

btw, thanks a lot for the discussion and your work

…fail when spawning processes (cf: winfinit#25, mongodb-js#25 & mccormicka/Mockgoose#207)

winfinit · 2017-03-08T18:10:38Z

fork is not used anymore with most recent rewrite

spawn sync only with --fork

d8fa505

ralflizard added a commit to ralflizard/mongodb-prebuilt that referenced this pull request May 30, 2016

Applied mongodb-js#25 for Mockgoose startup on Windows

5a71145

almeidap added a commit to admin-ch/mongodb-prebuilt that referenced this pull request Oct 12, 2016

Ensure that starting server under Windows through Mockgoose does not …

cad20d7

…fail when spawning processes (cf: winfinit#25, mongodb-js#25 & mccormicka/Mockgoose#207)

winfinit closed this Mar 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

spawn sync only with --fork #25

spawn sync only with --fork #25

pibi commented May 9, 2016 •

edited

Loading

seriousManual commented May 10, 2016

winfinit commented May 10, 2016

pibi commented May 10, 2016

winfinit commented May 10, 2016

pibi commented May 11, 2016 •

edited

Loading

winfinit commented May 11, 2016

pibi commented May 11, 2016 •

edited

Loading

winfinit commented May 11, 2016

pibi commented May 11, 2016 •

edited

Loading

pibi commented May 11, 2016

winfinit commented May 11, 2016

pibi commented May 11, 2016 •

edited

Loading

winfinit commented Mar 8, 2017

spawn sync only with --fork #25

spawn sync only with --fork #25

Conversation

pibi commented May 9, 2016 • edited Loading

seriousManual commented May 10, 2016

winfinit commented May 10, 2016

pibi commented May 10, 2016

winfinit commented May 10, 2016

pibi commented May 11, 2016 • edited Loading

winfinit commented May 11, 2016

pibi commented May 11, 2016 • edited Loading

winfinit commented May 11, 2016

pibi commented May 11, 2016 • edited Loading

pibi commented May 11, 2016

winfinit commented May 11, 2016

pibi commented May 11, 2016 • edited Loading

winfinit commented Mar 8, 2017

pibi commented May 9, 2016 •

edited

Loading

pibi commented May 11, 2016 •

edited

Loading

pibi commented May 11, 2016 •

edited

Loading

pibi commented May 11, 2016 •

edited

Loading

pibi commented May 11, 2016 •

edited

Loading