nwjs / nw.js

Call all Node.js modules directly from DOM/WebWorker and enable a new way of writing applications with all Web technologies.
https://nwjs.io
MIT License
40.33k stars 3.88k forks source link

child_process.fork() issue #213

Closed rogerwang closed 10 years ago

rogerwang commented 11 years ago

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

zcbenz commented 11 years ago

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 commented 11 years ago

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: https://github.com/rogerwang/node-webkit/issues/213#issuecomment-10677284

zcbenz commented 11 years ago

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 commented 11 years ago

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: https://github.com/rogerwang/node-webkit/issues/213#issuecomment-10677961

rogerwang commented 11 years ago

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 commented 11 years ago

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

ghost commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

:+1:

fritx commented 10 years ago

:+1:

cognitom commented 10 years ago

:+1:

shabeepk commented 10 years ago

:+1:

mrliptontea commented 10 years ago

:+1:

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

shabeepk commented 10 years ago

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: :+1:]

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 https://github.com/rogerwang/node-webkit/issues/213#issuecomment-45445768 .

mrliptontea commented 10 years ago

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 commented 10 years ago

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

rogerwang commented 10 years ago

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

fmartingr commented 10 years ago

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

sayjava commented 10 years ago

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.

rogerwang commented 10 years ago

The fix can be tested now with:

http://dl.node-webkit.org/live-build/08-25-2014/win64_master-build-153-5e94e1f-1cc177b-731b9e2-0f7c844-3d94950-86728d1/node-webkit-v0.10.2-win-ia32.zip http://dl.node-webkit.org/live-build/08-25-2014/mac64_master-build-153-5e94e1f-1cc177b-731b9e2-0f7c844-3d94950-86728d1/node-webkit-v0.10.2-osx-x64.zip http://dl.node-webkit.org/live-build/08-25-2014/linux64_master-build-150-5e94e1f-1cc177b-731b9e2-0f7c844-3d94950-86728d1/node-webkit-v0.10.2-linux-x64.tar.gz

studiochris commented 10 years ago

Thanks Roger. Seems to be working on my end.

Mithgol commented 10 years ago

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 commented 10 years ago

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

malpower commented 10 years ago

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

symunona commented 8 years ago

Hi! This feature still does not seem to work. I have tried the example here: https://github.com/nwjs/nw.js/issues/1575#issuecomment-35407665

It gives undefined as described.

Does anyone have a working example code?

Or alternatives? I have tried now:

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 commented 8 years ago

@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.