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

Have julialauncher use execve() instead of launching a new process #552

Closed
staticfloat opened this issue Jan 13, 2023 · 7 comments · Fixed by #569
Closed

Have julialauncher use execve() instead of launching a new process #552

staticfloat opened this issue Jan 13, 2023 · 7 comments · Fixed by #569
Labels
enhancement New feature or request
Milestone

Comments

@staticfloat
Copy link
Sponsor Member

I think we should change julialauncher to use execve() (or its moral equivalent, wherever possible) instead of fork() when launching the julia sub-process. Because this does not launch a child process but instead causes the current process to be replaced by the new process, this would have the following advantages:

  • gdb naturally follows exec() calls, whereas with fork() there's some ambiguity, so it has to be told to follow the child rather than stay with the parent.
  • Process listings would now show only one julia process, instead of two. (a minor benefit, but still nice)
  • We would no longer need to worry about passing exit codes back up; once we call execve() the rest is on the shoulders of the true julia application
  • We would no longer need to worry about signal handlers.
@davidanthoff davidanthoff added the enhancement New feature or request label Jan 13, 2023
@davidanthoff davidanthoff added this to the Backlog milestone Jan 13, 2023
@davidanthoff
Copy link
Collaborator

This would be Linux/Mac only, right? I don't think Windows has something like this?

And the second question is: right now julialauncher spawns the real julia process async, and then does other things, in particular check for new versions, while it waits for the julia process to finish. Could we still do that?

@staticfloat
Copy link
Sponsor Member Author

This would be Linux/Mac only, right? I don't think Windows has something like this?

No, I don't think it does, unless @vtjnash knows of a secret execve-like function on Windows.

right now julialauncher spawns the real julia process async, and then does other things, in particular check for new versions, while it waits for the julia process to finish. Could we still do that?

The way I would do it is to have julialauncher fork() off a second process and then execve() the true julia executable.

@davidanthoff
Copy link
Collaborator

I think my primary worry is that this would essentially mean the Windows and Linux/Mac version would be more or less completely different... That doesn't strike me as ideal.

I think the exit codes and signal stuff is now handled properly? So I'm not too worried about that. But the gdb thing seems like a real benefit...

@maleadt
Copy link
Member

maleadt commented Jan 27, 2023

I just had a need for this as well, because NVIDIA's profilers don't seem to like the launch of a child process (and even if they would, they always require specific arguments like --target-processes=all). I did a quick experiment with:

let mut child_process = std::process::Command::new(julia_path)
    .args(&new_args)
    .exec();

... and that just works with the profilers I was testing. So a vote for using execve from me 🙂

I think my primary worry is that this would essentially mean the Windows and Linux/Mac version would be more or less completely different... That doesn't strike me as ideal.

We could use the fork code path for Windows too (behind the scenes using CreateProcess of course). And the signal handling is Linux/Mac only already.

@StefanKarpinski
Copy link
Sponsor Member

This seems like a good idea to me as well and I don't think the deviation between Windows and UNIX would be so bad—it could be very contained. Everything is the same right up until the moment of starting the julia process and then:

  • UNIX: fork a background process and do execve to replace the current process with julia
  • Windows: start julia as child process and then do the background stuff while waiting for it

In both cases you end up with some code running to do background stuff and the only other difference is whether to wait on julia as a child process and propagate its status (UNIX: no; Windows: yes). Sure, these are different, but it's a very contained difference and the way child process are started is radically different between these OSes anyway.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 28, 2023

On windows you can pick a parent process and get status on anything, if you care enough to

@StefanKarpinski
Copy link
Sponsor Member

Let's have this discussion over here instead of on a tangentially related (closed) issue. @davidanthoff wrote:

Yeah, I'm just quite nervous about the interaction of execve and Rust packages, there seems to be a lot of potential for pain. And on Windows that won't ever work, and I'm also not keen to have such a massive difference there...

My sense right now is that the only way to really have a one-process solution that is architecturally similar on the various platforms would be some variant of #327. Which also seems anything but trivial...

I don't want a one process solution—I'm happy to have julialauncher spawn a separate child process to do updates. But I do think that it's strictly better if the julialauncher process replaces itself with the actual julia process that the user wants to run. Can you elaborate on what the possible Rust issue might be? Once the process has called execve I don't see problems Rust can possibly cause since the process has been replaced. I do get the desire to have all platforms work the same, but it seems like the difference in logic can be kept pretty small and at this point we have enough users on all platforms that I think we'll know if something isn't working on one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants