Is this a bug in dash? I quick search hasn't found any evidence that it should not pass signals to spawned processes.
No i think its the combination of dash and the node.js implementation of how child.process works and how it sends a signal to the process not one or the other.
So i think its not really a bug but rather a kind of incompatibility.
I see that the shell command also accepts boolean true and false - does setting that to either have an effect ?
Wasnât aware of this as this is what is in the child.process documentation
Edit Ok just checked again. @dceejay the boolean option is just for execFile and spawn. When using either it only uses a shell when set to true. Exec itself always uses a shell so it doesnât have the boolean. But i guess it be better staying with exec as booth spawn and execFile donât support piping and stuff in entered commands.
1 Like
I opened an issue on github but itâs going to be hard to be fixed if at all. There is a few links in the issue to node.js and underlying library discussion about the whole complex around the inconsistency in shell behaviour on different platforms and with different shells.
It doesnât seem that this will be fixed in node as they pretty much agreed to just mention this behavior in the docs somewhere and leave it at that as any possible solution would revolve around so many external factors as not being practical in the end while keeping cross platform compatibility. So maybe just adding a warning to the docs about some shells and that using spawn will be the better way is the easiest or only way.
Edit:
@knolleary i added a possible workaround as a comment to the issue. It would check if on linux and bash is present and in this case add the shell arg to the options if you want to have a look.
Could the node look for the spawned process and kill that explicitly before killing the shell?
Have a look in the issue i linked. I posted an idea how to maybe workaround it most of the times by checking if bash is there and if it is use it.
Yes, that could be a solution, provided /bin/bash is present. but in the more general case might there be systems which have dash and not bash?
Yes thatâs why itâs only a workaround. The bash shell argument would only be used if itâs linux and bash is present.
In rare cases where there is only dash it would be used and it would just be like now. Itâs just that that would be the case in way less cases and i think bash is always installed on ubuntu and debian at least as it is still the standard login shell, just not what /bin/sh is pointing too. It would at least solve this problem for the many people running nodered on debian or ubuntu based systems.
Would my suggestion of killing the spawned process explicitly not be a general solution?
But you would also need to kill the shell. And its not that easy anyway as you need the pid of the process you want to kill. You only get the pid of the shell and not the actual process within node.js with the child.pid property when using exec. Mostly the process pid itself is the shell pid + 1 but not always depending on the process.
So you re back at an even uglier workaround trying to blindly also kill pid + 1 as their is no direct way to know the pid of the process that will be orphaned. And you'd be doing this although not necessary in many if not most cases as it seems to be a dash/debian/ubuntu specific behaviour.
Edit:
If you have a simple elegant solution id be more than happy to see it but from my point of view for implementing your proposal it wouldnt be 3 lines of code but many more as you would have to implement this in not one place but two or three within the exec nodes code. And it would add a lot of logic and code overhead as you would actually probably want to start checking if the pid + 1 actually exists before killing and youd need to require another external module like https://www.npmjs.com/package/is-running or do a whole additional exec.
I just think it would add a whole lot of complexity and im not sure if that is wanted for a core node.
Im sorry if this comes across as stubborn here but i dont see an easy general solution to this.
It would have to use ps to get the spawned process id. I am not saying this is necessarily the right way to go, but it is an option I think. The bash test technique is certainly simpler and would fix the vast majority of cases.
1 Like
I added some additional thoughts to the previous post.
Maybe you can have a go at writing some code that would do your fix and add it to the github issue above 
one way or another the state right now is not preferable.
I don't think it would be quite as complex as you think, as the shell pid is already known (it is shown in the status) and ps can provide the pid of processes spawned by that pid so no need to assume it is pid + 1 (which would not be appropriate as it cannot be guaranteed), but it would need the ps module installed and some non-trivial code, much more than a few lines.
Not stubborn at all, it is always good to thrash alternative ideas around. The spawned pid solution would, I think, be a general solution but it would be significantly more complex than your idea and, as I said, yours would sort it for the vast majority of cases (possibly even all of those who would ever come across it) so I would be happy to see your suggestion implemented.
1 Like
This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.
