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

Process spawning via PATH is different on unix than it is on windows #15149

Closed
alexcrichton opened this issue Jun 24, 2014 · 3 comments · Fixed by #16284
Closed

Process spawning via PATH is different on unix than it is on windows #15149

alexcrichton opened this issue Jun 24, 2014 · 3 comments · Fixed by #16284
Labels
O-windows Operating system: Windows

Comments

@alexcrichton
Copy link
Member

Due to the separate fork + exec steps on unix, the following test case passes on unix. On windows, however, the "atomic" fork + exec causes the test to fail.

Notably, on unix the child's PATH environment variable is used to lookup the program to execute, whereas on windows it uses the parent's PATH variable.

use std::os;
use std::io::{File, TempDir, Command, util, UserRWX, fs};

fn main() {
    let args = os::args();
    if args.len() > 1 && args.get(1).as_slice() == "child" {
        return assert_eq!(args.get(0).as_slice(), "mytest");
    }

    let dir = TempDir::new("mytest").unwrap();
    let me = os::self_exe_name().unwrap();
    let dest = dir.path().join("mytest");
    util::copy(&mut File::open(&me), &mut File::create(&dest)).unwrap();
    fs::chmod(&dest, UserRWX).unwrap();

    Command::new("mytest").env(&[("PATH".to_string(),
                                  dir.path().as_str().unwrap().to_string())])
                          .arg("child")
                          .spawn().unwrap();
}

The behavior of the two platforms needs to be consistent, and I believe that the unix behavior is what one would expect.

@alexcrichton
Copy link
Member Author

Note that the test is just copying itself to a temporary directory and then running it from the temporary directory via PATH (cutting through the irrelevant bits)

@wycats
Copy link
Contributor

wycats commented Jun 24, 2014

The Unix behavior sounds right to me.

@alexcrichton
Copy link
Member Author

I have a proposed fix with libuv to fix the libgreen side of things: joyent/libuv#1337

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jun 25, 2014
* Add a convenience method bin() for generating the name of a binary. On windows
  this remembers to append `.exe`.

* Stop executing relative paths to binaries and relying on PATH. This is
  suffering from rust-lang/rust#15149 and failing to spawn processes on windows.
  Additionally, this allows the tests to work with a pre-installed cargo becuase
  the freshly built executables are precisely specified.

* A new function, escape_path(), was added for tests. When generated source
  files with paths, this function needs to be called to properly escape the
  \-character that appears in windows path names. Without this function we would
  be generating invalid TOML and rust.
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 13, 2014
In order to have the spawning semantics be the same for unix/windows, the
child's PATH environment variable needs to be searched rather than the parent's
environment variable.

If the child is inheriting the parent's PATH, then no action need be taken as
windows will do the heavy lifting. If the child specifies its own PATH, then it
is searched beforehand for the target program and the result is favored if a hit
is found.

cc rust-lang#15149, but does not close the issue because libgreen still needs to be
updated.
bors added a commit that referenced this issue Jul 13, 2014
In order to have the spawning semantics be the same for unix/windows, the
child's PATH environment variable needs to be searched rather than the parent's
environment variable.

If the child is inheriting the parent's PATH, then no action need be taken as
windows will do the heavy lifting. If the child specifies its own PATH, then it
is searched beforehand for the target program and the result is favored if a hit
is found.

cc #15149, but does not close the issue because libgreen still needs to be
updated.
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 6, 2014
There was a bug in both libnative and libuv which prevented child processes from
being spawned correctly on windows when one of the arguments was an empty
string. The libuv bug has since been fixed upstream, and the libnative bug was
fixed as part of this commit.

When updating libuv, this also includes a fix for rust-lang#15149.

Closes rust-lang#15149
Closes rust-lang#16272
bors added a commit that referenced this issue Aug 12, 2014
There was a bug in both libnative and libuv which prevented child processes from
being spawned correctly on windows when one of the arguments was an empty
string. The libuv bug has since been fixed upstream, and the libnative bug was
fixed as part of this commit.

When updating libuv, this also includes a fix for #15149.

Closes #15149
Closes #16272
alexcrichton added a commit to alexcrichton/cargo that referenced this issue Sep 2, 2014
* Add a convenience method bin() for generating the name of a binary. On windows
  this remembers to append `.exe`.

* Stop executing relative paths to binaries and relying on PATH. This is
  suffering from rust-lang/rust#15149 and failing to spawn processes on windows.
  Additionally, this allows the tests to work with a pre-installed cargo becuase
  the freshly built executables are precisely specified.

* A new function, escape_path(), was added for tests. When generated source
  files with paths, this function needs to be called to properly escape the
  \-character that appears in windows path names. Without this function we would
  be generating invalid TOML and rust.
flaper87 added a commit to flaper87/rust that referenced this issue Jan 23, 2015
While trying to experiment with changes for some other issues, I noticed that the test for rust-lang#15149 was failing because I have `/tmp` mounted as `noexec` on my Linux box, and that test tries to run out of a temporary directory. This may not be the most common case, but it's not rare by any means, because executing from a world-writable directory is a security problem. (For this reason, some kernel options/mods such as grsecurity also can prevent this on Linux.) I instead copy the executable to a directory created in the build tree, following the example of the `process-spawn-with-unicode-params` test.

After I made that change, I noticed that I'd made a mistake, but the test was still passing, because the "parent" process was not actually checking the status of the "child" process, meaning that the assertion in the child could never cause the overall test to fail. (I don't know if this has always been the case, or if it has something to do with either Windows or a change in the semantics of `spawn`.) So I fixed the test so that it would fail correctly, then fixed my original mistake so that it would pass again.

The one big problem with this is that I haven't set up any machines of my own so that I can build on Windows, which is the platform this test was targeted at in the first place! That might take a while to address on my end. So I need someone else to check this on Windows.
flaper87 added a commit to flaper87/rust that referenced this issue Jan 24, 2015
While trying to experiment with changes for some other issues, I noticed that the test for rust-lang#15149 was failing because I have `/tmp` mounted as `noexec` on my Linux box, and that test tries to run out of a temporary directory. This may not be the most common case, but it's not rare by any means, because executing from a world-writable directory is a security problem. (For this reason, some kernel options/mods such as grsecurity also can prevent this on Linux.) I instead copy the executable to a directory created in the build tree, following the example of the `process-spawn-with-unicode-params` test.

After I made that change, I noticed that I'd made a mistake, but the test was still passing, because the "parent" process was not actually checking the status of the "child" process, meaning that the assertion in the child could never cause the overall test to fail. (I don't know if this has always been the case, or if it has something to do with either Windows or a change in the semantics of `spawn`.) So I fixed the test so that it would fail correctly, then fixed my original mistake so that it would pass again.

The one big problem with this is that I haven't set up any machines of my own so that I can build on Windows, which is the platform this test was targeted at in the first place! That might take a while to address on my end. So I need someone else to check this on Windows.
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 17, 2023
internal: Speedup line index calculation via SSE2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants