Skip to content

Support :chdir in #spawn #1492

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

Closed
MaxLap opened this issue Nov 30, 2018 · 7 comments
Closed

Support :chdir in #spawn #1492

MaxLap opened this issue Nov 30, 2018 · 7 comments
Assignees
Milestone

Comments

@MaxLap
Copy link

MaxLap commented Nov 30, 2018

Right now, spawn('echo hi', chdir: '/home') is plain refused in TruffleRuby.

truffleruby 1.0.0-rc9, like ruby 2.4.4, GraalVM CE Native [x86_64-linux]

MaxLap added a commit to deep-cover/deep-cover that referenced this issue Nov 30, 2018
@nirvdrum
Copy link
Collaborator

Thanks for the report! This is unfortunately a known limitation in TruffleRuby. Incidentally, we were just discussing what we can do about it a few days back. The core issue is we don't support fork. The posix_spawnp call doesn't provide a mechanism to change directories when executing a call, so MRI forks, changes the directory, then execs.

Unfortunately, approaches like changing the directory, performing the spawn, and then changing back to the previous directory suffer from race conditions in the presence of multiple running threads.

Anyway, it's something for us to sort out and we're aware we need to. I'm hopeful we'll work out a solution soon.

@MaxLap
Copy link
Author

MaxLap commented Nov 30, 2018

I understand, I just didn't see any mention of it in the doc so I thought I'd report it. It's not obvious that not supporting fork means no :chdir. At least someone searching may find this explanation now.

I wish I could help, but other than providing a random workaround that you guys probably already thought of and discarded, I got nothing. (Can't you #spawn a binary/non-ruby script which receives the information, does the chdir, and thenexec the real target?)

@nirvdrum
Copy link
Collaborator

Your suggestion is what we've converged on. One of us will try that approach out next week. Not shipping an extra binary would be preferable, but we were unable to come up with a reasonable solution otherwise. "Reasonable" here means not incurring a bunch of overhead.

MaxLap added a commit to deep-cover/deep-cover that referenced this issue Nov 30, 2018
MaxLap added a commit to deep-cover/deep-cover that referenced this issue Dec 2, 2018
MaxLap added a commit to deep-cover/deep-cover that referenced this issue Dec 2, 2018
MaxLap added a commit to deep-cover/deep-cover that referenced this issue Dec 6, 2018
MaxLap added a commit to deep-cover/deep-cover that referenced this issue Dec 6, 2018
@eregon eregon self-assigned this Jan 7, 2019
@eregon
Copy link
Member

eregon commented Jan 7, 2019

I'll try to take a look at this this week.

MaxLap added a commit to deep-cover/deep-cover that referenced this issue Mar 20, 2019
@eregon
Copy link
Member

eregon commented Aug 22, 2019

FWIW JRuby uses cd workdir && ... but that invokes the shell (can be slow) and if arguments are passed as an Array they would need to be escaped into a flat command line.

I think the best approach here is having a small executable doing chdir + execve, much like Java does in some cases for ProcessBuilder.

It seems posix_spawn_file_actions_addchdir_np was added in glibc 2.29: https://sourceware.org/ml/libc-announce/2019/msg00000.html
But that might take a while until all OS have a recent enough glibc.
We could still check if it is available and use it as an optimization, though.

@eregon eregon added this to the 19.3.0 milestone Aug 26, 2019
@eregon
Copy link
Member

eregon commented Aug 26, 2019

I fixed this in d39f45e and it will be in the next release.
@MaxLap Thank you for reporting the bug, and sorry it took so long to actually fix it.

@eregon eregon closed this as completed Aug 26, 2019
@eregon
Copy link
Member

eregon commented Nov 22, 2019

FYI, TruffleRuby 19.3.0 was released and should fix this.

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

No branches or pull requests

3 participants