Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child_process.fork() issue #213

Closed
rogerwang opened this issue Nov 24, 2012 · 34 comments
Closed

child_process.fork() issue #213

rogerwang opened this issue Nov 24, 2012 · 34 comments
Assignees

Comments

@rogerwang
Copy link
Member

child_process module from Node is broken. This blocks Node applications offloading work to another process.

@ghost ghost assigned rogerwang Nov 24, 2012
@zcbenz
Copy link
Contributor

zcbenz commented Nov 24, 2012

Does every method of child_process break for you or just one of the methods is broken? I can only confirm child_process.fork is broken.

@rogerwang
Copy link
Member Author

yeah I mean fork.

-------- Original Message --------
From: Cheng Zhao notifications@github.com
Sent: Sat Nov 24 20:45:20 格林尼治标准时间+0800 2012
To: rogerwang/node-webkit node-webkit@noreply.github.com
Cc: Roger Wang wenrui@gmail.com
Subject: Re: [node-webkit] child_process module is broken (#213)

Does every method of child_process break for you or just one of the methods is broken? I can only confirm child_process.fork is broken.


Reply to this email directly or view it on GitHub:
#213 (comment)

@zcbenz
Copy link
Contributor

zcbenz commented Nov 24, 2012

The implementation of child_process.fork is just to execute node itself with path to script as its parameters:

return spawn(process.execPath, args, options);

It failed in nw because we don't support to execute a .js file. Even if we do, the cost of child_process.fork would be quite expensive.

A quick fix of it is to distribute a node.exe along with nw.exe, and replace process.execPath with:

var path = require('path');
path.join(path.dirname(process.execPath), 'node.exe')

@rogerwang
Copy link
Member Author

that's only a workaround. nw.exe should have a switch to behave in the same way as upstream node.exe

-------- Original Message --------
From: Cheng Zhao notifications@github.com
Sent: Sat Nov 24 22:06:11 格林尼治标准时间+0800 2012
To: rogerwang/node-webkit node-webkit@noreply.github.com
Cc: Roger Wang wenrui@gmail.com
Subject: Re: [node-webkit] child_process.fork() is broken (#213)

The implementation of child_process.fork is just to execute node itself with path to script as its parameters:

return spawn(process.execPath, args, options);

It failed in nw because we don't support to execute a .js file. Even if we do, the cost of child_process.fork would be quite expensive.

A quick fix of it is to distribute a node.exe along with nw.exe, and replace process.execPath with:

var path = require('path');
path.join(path.dirname(process.execPath), 'node.exe')

Reply to this email directly or view it on GitHub:
#213 (comment)

@rogerwang
Copy link
Member Author

This patch on src/third_party/node is the workaround, tested with node-compute-cluster:

diff --git a/lib/child_process.js b/lib/child_process.js
index 9b31586..bf200d6 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -444,7 +444,12 @@ exports.fork = function(modulePath /*, args, options*/) {
   options.stdio = options.silent ? ['pipe', 'pipe', 'pipe', 'ipc'] :
       [0, 1, 2, 'ipc'];

-  return spawn(process.execPath, args, options);
+  var path = require('path');
+  var nodePath =   path.join(path.dirname(process.execPath), 'node');
+  if (process.platform === 'win32') {
+    nodePath += ".exe";
+  }
+  return spawn(nodePath, args, options);
 };

@YurySolovyov
Copy link

is it going to be fixed to get default node behavior?
like var worker = child_process.fork('worker.js')

@ghost
Copy link

ghost commented Feb 15, 2014

Please change title of this issue. Just reading the issue gives a false impression about
child_process.fork. I know this may have been a problem over year ago. But I am a new-be to node and node-webkit. There is good information here, but the title has too negative message.

@gunnar-t
Copy link

The issue in #1575 (please excuse the missleading title) is new (imho). Now, with the switch from 0.8.4 to 0.9.1 even the workaround or quick fix provided by zcbenz by adding the execPath parameter to fork is broken.
Now there is now way to have some worker (childs, whatever) to do some heavy work which provides the nodejs functionality.

Is it planned (in some future release) to add nodejs functionality to WebWorker or at least to fix fork in any way?

Again, I think this feature is crucial to some applications, to spread heavy work to child processes.

Cheers Gunnar

P.S. Sorry for pushing that hard, but I am working at a program in my spare time since 5 month and with this feature broken, I can throw the whole thing in the trash :) (Or switch to some other product ....)

@YurySolovyov
Copy link

@gunnar-t I'm so agree with you. Fast and responsive apps need to offload work to worker processes to achieve good performance, but regular workers is not enough, node functionality is essential.

@gunnar-t
Copy link

gunnar-t commented Mar 1, 2014

@rogerwang Could you please give any hint wether you plan to fix this error or not? Or maybe how hard it is to fix.

Without any improvement here, I need to switch to another desktop nodejs solution.
Thanks in advance.

@studiochris
Copy link

Would appreciate an update on this as well. For now, I can't move on to 0.9 branch to take advantage of its new features and speed improvements.

@programmarchy
Copy link

Bump. I tried the "include node binary" hack above, but it didn't work for me ("Uncaught Error: channel closed", source: events.js (72)) and I think it could be because my worker uses modules that have a native bindings, which are built with nw-gyp instead of node-gyp.

My use case: doing a bluetooth inquiry with node-bluetooth-serial-port, which has native bindings to target the host OS bluetooth stack. The inquiry has to run on the main thread, which is fine except it freezes node-webkit's UI. So, I'm forking my bluetooth scanner and having it stream inquiry results back to me, then running into this bug.

@gunnar-t
Copy link

gunnar-t commented Apr 6, 2014

Hey Roger, as you can see, there are lots of people that would like to use that feature. Please can you tell anything concering that matter?!

Do I really have to switch to a library like messenger.js and build a communication between node-webkit and a completely different node.js process just to use "a worker-like" behaviour. That's sad.... :(

@rogerwang
Copy link
Member Author

I'd like to fix this as soon as I can. But I don't have a date. If you don't want to wait, I suggest finding commercial support in the community.

@gunnar-t
Copy link

Hi Roger, thank you for your answer. The important things was, that this feature will be fixed in the future. Right now I am fine with version 0.8.X .... Since my project will be commercial someday (maybe) I just needed the certainty to be able to update the underlying stack at a future date.

Thanks again for your work!

@dynamite-ready
Copy link

Just to clarify, the workaround involves embedding a Node JS runtime executable to call at runtime...
Are there any code examples out there please?

@sindresorhus
Copy link

👍

2 similar comments
@fritx
Copy link

fritx commented May 17, 2014

👍

@cognitom
Copy link

👍

@shabeepk
Copy link

shabeepk commented Jun 7, 2014

👍

@mrliptontea
Copy link

👍

plus: some claim it worked in 0.8.4, but I didn't have any luck with that version either.

@shabeepk
Copy link

shabeepk commented Jun 8, 2014

It does work for me on 0.8.4. However if you are on windows then you need
to fork with silent: true
On Jun 9, 2014 12:24 AM, "mrliptontea" notifications@github.com wrote:

[image: 👍]

plus: some claim it worked in 0.8.4, but I didn't have any luck with that
version either.


Reply to this email directly or view it on GitHub
#213 (comment)
.

@mrliptontea
Copy link

Yes, you're right, it does work in 0.8.4.
That was just my silly mistake. I downloaded 0.8.4 binaries and tried to launch with drag and drop and that way path ./ was pointing to place where nw.exe was, not to app's cwd.

That being said, I can now confirm it works up to 0.8.6.

@danieleds
Copy link

Hi Roger, can you give us an hint on where to look to fix this?

@rogerwang
Copy link
Member Author

@danieleds I'll look to fix soon. The first thing to do is to let node-webkit behave like upstream node. Thanks

@fmartingr
Copy link

I can't get it to work. Any example is really appreciated. Hope we get web workers with nodejs soon enough :D

@sayjava
Copy link

sayjava commented Aug 16, 2014

I can't wait for this issue to be fixed, it makes it hard to build anything useful if backgrounding is broken. Your effort is very appreciated.

@studiochris
Copy link

Thanks Roger. Seems to be working on my end.

@Mithgol
Copy link
Contributor

Mithgol commented Sep 3, 2014

For future references: it works for Linux in node-webkit v0.10.3, but not on Windows.

Details: https://groups.google.com/d/msg/node-webkit/UIh7RMNk9pQ/m2msPgSa0X4J

@malpower
Copy link

malpower commented Sep 8, 2014

yep, it is not work on windows, hope it will be fixed very soon.

rogerwang added a commit to nwjs/node that referenced this issue Sep 25, 2014
@malpower
Copy link

malpower commented Oct 6, 2014

finally, it's work on my windows 32bit machine.

@symunona
Copy link

symunona commented Jun 16, 2016

Hi! This feature still does not seem to work. I have tried the example here: #1575 (comment)

It gives undefined as described.

Does anyone have a working example code?

Or alternatives? I have tried now:

  • webworkify - does not work
  • webworker in the background (I need fs, parsing big files), does not work:
    In app.js:
var cp = require('child_process');
function fork() {
   var p = cp.fork('worker');\
}

It says: "Failed to load extension from ... Manifest file is missing or unreadable."

So I created a manifest.json file (I doubt that it should work like that tough) - still nothing - now I get a notification that "new background process is added" - I do not think that this is what I need.

I might get something wrong but I just want to run a long-time parser in the thread.

The problem with this solution for me though: I have to restructure my parsing code in order to make it break some time, so the UI will actually refresh, and it's pretty bad.

Is there any suggestions / workarounds, what I am missing, how I could just run something, uses the file system to export, and just lets the main thread know when it's done? The ugly version is that I not only package nw into enigma, but putting node next to it, so it can call the parser separately - any other ideas?

I am using nwjs-sdk-v0.15.2-win-x64 on windows 10

Thanks!

@polpo
Copy link

polpo commented Jun 29, 2016

@symunona NW.js detects if it needs to run as node by checking if the given argument has a .js extension. So name your worker worker.js and change the fork call to cp.fork('worker.js'); and all should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests