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

rdctl: Don't use process groups on Linux #7965

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

mook-as
Copy link
Contributor

@mook-as mook-as commented Dec 17, 2024

Disable killing all processes in the process on Linux, as Electron does not create a new process group when running from the RPM/deb package. Leave it using process groups on macOS, as launchd still does it.

It is expected that we will revert this once we sort out process groups on Linux.

Fixes #7945

@mook-as mook-as force-pushed the rdctl/linux-no-process-group branch 2 times, most recently from 4fdc6c1 to a39654e Compare December 18, 2024 00:29
Disable killing all processes in the process on Linux, as Electron does not
create a new process group when running from the RPM/deb package.  Leave it
using process groups on macOS, as launchd still does it.

It is expected that we will revert this once we sort out process groups on
Linux.

Signed-off-by: Mark Yen <[email protected]>
@mook-as mook-as force-pushed the rdctl/linux-no-process-group branch from a39654e to ea31ee9 Compare December 18, 2024 01:55
@mook-as mook-as marked this pull request as ready for review December 18, 2024 02:12
@gunamata gunamata requested a review from Nino-K December 18, 2024 17:52
@@ -39,6 +40,12 @@ exit, and once it does, terminates all processes within the same process group.`
if err != nil {
return fmt.Errorf("failed to get process ID: %w", err)
}
if runtime.GOOS == "linux" {
// TODO: We can't use the process group on Linux, because Electron does
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need an issue for these TODOs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; filed #7972 for that.

@Nino-K Nino-K merged commit 4079bff into rancher-sandbox:main Dec 18, 2024
20 checks passed
@mook-as mook-as deleted the rdctl/linux-no-process-group branch December 18, 2024 21:12
@mook-as mook-as restored the rdctl/linux-no-process-group branch December 18, 2024 21:12
@mook-as mook-as deleted the rdctl/linux-no-process-group branch December 24, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exiting the app makes the screen go blank on Linux
2 participants