Feature #2715: Optimization to avoid spawning shell in Kernel#system call should check for failure conditions

• Status: Feedback
• Assignee: -
• Target version: -

[taw (Tomasz Wegrzanowski)]

=begin
This is an old issue, I think going all the way back to Perl's system().

Kernel#system is supposed to spawn shell and pass its argument to shell process just like C system(); except it's optimized for a common case of very simple argument, in which case Ruby (like Perl) simply does fork and exec of the string passed, split on whitespace.

This almost works, except shell error reporting is not duplicated. If command doesn't exist, shell would print an error message on stderr, while Ruby like Perl fails silently.

Problem is present in all versions of Ruby including 1.8.7 and 1.9.1.

To reproduce:
$ ruby -e 'system "fail"'
$ ruby -e 'system """fail"'
sh: fail: command not found

Output of both commands should be identical (or at least close enough, details of error message might differ by Unix flavor).

rb_f_system code for VMS and Windows uses different code paths, so I don't know if it's also affected or not.

I wrote a bit more about background of this issue on my blog: http://t-a-w.blogspot.com/2010/02/what-is-all-this-perl-doing-in-my-ruby.html
=end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Feedback

=begin

=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Sat, 6 Feb 2010 06:55:09 +0900,
Tomasz Wegrzanowski wrote in [ruby-core:28072]:

This almost works, except shell error reporting is not
duplicated. If command doesn't exist, shell would print an
error message on stderr, while Ruby like Perl fails silently.

Problem is present in all versions of Ruby including 1.8.7 and 1.9.1.

To reproduce:
$ ruby -e 'system "fail"'
$ ruby -e 'system """fail"'
sh: fail: command not found

Output of both commands should be identical (or at least
close enough, details of error message might differ by Unix
flavor).

What's that you want to do? In 1.9, the former returns nil,
while the later returns false, and spawn and Process.spawn
raise an Errno::ENOENT.

In addtion, system and spawn in 1.9 have been improved very
much, so you don't need to use sh for redirection.

run "fail" command with no output

system("fail", out:"/dev/null", err:[:child, :out])

I wrote a bit more about background of this issue on my blog:
http://t-a-w.blogspot.com/2010/02/what-is-all-this-perl-doing-in-my-ruby.html

Please describe the rationale briefly on the BTS, not separated
blog.

--
Nobu Nakada

=end

[taw (Tomasz Wegrzanowski)]

=begin
Nobu: I want system "fail" and system "''fail" to behave identically - printing error message when command fail doesn't exist.

For Ruby's system() not to spawn shell should be just performance optimization, it should behave the same way regardless of it spawns shell (system "''fail") or not (system "fail").

That's all.
=end

[mame (Yusuke Endoh)]

=begin
Hi,

For Ruby's system() not to spawn shell should be just performance optimization

I don't think so. Indeed, it is a feature for optimization, but
it is also spec.

Process invocation may cause significant result such as breaking
system. So changing that may cause compatibility issue.

Nobu: I want system "fail" and system "''fail" to behave identically - printing error message when command fail doesn't exist.

"Printing error message" is absolutely new feature.
I move this ticket to feature tracker, though the feature request
is really uncool for me...

--
Yusuke Endoh [e-mail]
=end

[akr (Akira Tanaka)]

• Project changed from Ruby to Ruby master
• Category changed from core to core

[akr (Akira Tanaka)]

• Description updated (diff)

I feel the optimization (avoid shell when no meta characters) can be removed.

We have many ways to avoid shell such as system(command, arg1, arg2).

And recent computers are fast enough to run command via shell.

[mame (Yusuke Endoh)]

• Assignee set to akr (Akira Tanaka)

[akr (Akira Tanaka)]

• File remove-shell-invocation-optimization.patch remove-shell-invocation-optimization.patch added

I wrote a patch to remove the optimization:
remove-shell-invocation-optimization.patch

However it was not good enough because it causes several test failures.

Several failures are caused by pid change which the invoked command is not
direct child process of ruby process.
(This depends on /bin/sh. If /bin/sh optimizes tail invocations,
invoked command process will be a direct child process.)

I think this is incompatible enough.

Now, I feel printing a error message suggested by taw is better solution.

[kosaki (Motohiro KOSAKI)]

Hmm...

As akr-san described, some shells have tail invocation optimization. Thus, even if ruby doesn't have shell invocation optimization, caller can't assume that spawned process is neither child nor grand child.

Therefore, if some tests were fail, it is certainly test mistake. I still think we can remove the shell invocation optimization. It is a source of confusion and I dislike it.

Of cource, it shouldn't be backported 1.9.x.

[akr (Akira Tanaka)]

As akr-san described, some shells have tail invocation optimization. Thus, even if ruby doesn't have shell invocation optimization, caller can't assume that spawned process is neither child nor grand child.

If we have shell invocation optimization (as now) and the command line is
simple, the command is spawned as a child process.

So removing the optimization breaks a program which assumes this behavior.

For example, IO.popen("foo bar") {|io| ... Process.kill sig, io.pid ... }
will be broken by removing the optimization.

I feel this is incompatible enough.

[kosaki (Motohiro KOSAKI)]

(6/5/12 3:55 PM), akr (Akira Tanaka) wrote:

Issue #2715 has been updated by akr (Akira Tanaka).

As akr-san described, some shells have tail invocation optimization. Thus, even if ruby doesn't have shell invocation optimization, caller can't assume that spawned process is neither child nor grand child.

If we have shell invocation optimization (as now) and the command line is
simple, the command is spawned as a child process.

So removing the optimization breaks a program which assumes this behavior.

For example, IO.popen("foo bar") {|io| ... Process.kill sig, io.pid ... }
will be broken by removing the optimization.

I feel this is incompatible enough.

.... I realized IO.pid is crazy mistaken feature. It wouldn't work when using
complex cmdline even if we will not remove the optimization. Sigh. (+_+

[akr (Akira Tanaka)]

.... I realized IO.pid is crazy mistaken feature. It wouldn't work when using complex cmdline even if we will not remove the optimization. Sigh. (+_+

Killing a complex command line, which invokes many processes, is a different problem.
IO#pid is not a origin of the problem.
spawn also have same problem as follows.

pid = spawn("foo & bar"); Process.kill(:TERM, pid)

Do you say that returning pid feature of spawn is crazy mistaken?

Killing a complex command line needs process group,
as shell job control does.

To return to this issue, I'm considering adding a new spawn option, :shell.
This option disables the optimization.

system("simple command", :shell=>true)

The default of the option is disabled as now.

This option solves this issue as:

• shell error messages are printed if :shell => true.
• doesn't break programs which expects simple commands are direct child process.
• don't need to implement mimic of shell error message printing feature in ruby.

[kosaki (Motohiro KOSAKI)]

On Fri, Jun 8, 2012 at 3:21 AM, akr (Akira Tanaka) [e-mail] wrote:

Issue #2715 has been updated by akr (Akira Tanaka).

.... I realized IO.pid is crazy mistaken feature. It wouldn't work when using complex cmdline even if we will not remove the optimization. Sigh. (+_+

Killing a complex command line, which invokes many processes, is a different problem.
IO#pid is not a origin of the problem.
spawn also have same problem as follows.

 pid = spawn("foo & bar"); Process.kill(:TERM, pid)

Do you say that returning pid feature of spawn is crazy mistaken?

Killing a complex command line needs process group,
as shell job control does.

To return to this issue, I'm considering adding a new spawn option, :shell.
This option disables the optimization.

 system("simple command", :shell=>true)

The default of the option is disabled as now.

This option solves this issue as:

• shell error messages are printed if :shell => true.
• doesn't break programs which expects simple commands are direct child process.
• don't need to implement mimic of shell error message printing feature in ruby.

Seems make sense.

[mame (Yusuke Endoh)]

• Target version set to 2.6

[naruse (Yui NARUSE)]

• Assignee deleted (akr (Akira Tanaka))
• Target version deleted (2.6)