Skip to content

Child processes remain after parent termination on Windows #49234

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

Open
slightfoot opened this issue Jun 10, 2022 · 7 comments
Open

Child processes remain after parent termination on Windows #49234

slightfoot opened this issue Jun 10, 2022 · 7 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@slightfoot
Copy link
Contributor

slightfoot commented Jun 10, 2022

This issue relates to process spawning and termination handling.

I have observed that child processes remain after parent termination only on Windows where-as on Mac and (I assume) Linux this terminates the entire child process tree when calling exit(0) for example in Dart.

This is especially important when spawning child dart processes that spawn more child dart processes like build and such. I have regularly seen a build up of running dart processes on my Windows system and I have to forcefully terminate them. I wonder if this is a manifestation of this underlying issue. And on Mac this is just not seen.

example

import 'dart:async';
import 'dart:io';

Future<void> main(List<String> args) async {
  final instance = (args.isNotEmpty) ? 'child' : 'parent';

  print('$instance started $pid');

  late Process process;
  if (instance == 'parent') {
    process = await Process.start('dart', [Platform.script.toString(), 'child']);
    process.stdout.pipe(stdout);
    process.stderr.pipe(stderr);
    stdin.pipe(process.stdin);
  }

  bool signalled = false;
  late StreamSubscription sub;
  sub = ProcessSignal.sigint.watch().listen((event) {
    sub.cancel(); // < https://github.com/dart-lang/sdk/issues/31264
    // process.kill(); // BAD
    signalled = true;
  });

  for (int i = 0; !signalled; i++) {
    print('$instance iteration $pid: $i');
    await Future.delayed(const Duration(seconds: 1));
  }

  print('$instance done: $pid');
}

Dart Mac Version: Dart SDK version: 2.17.1 (stable) (Tue May 17 17:58:21 2022 +0000) on “macos_arm64”
Running this on Mac and you get this (Pressing Ctrl+C part way through):

# dart main.dart
parent started 13807
parent iteration 13807: 0
child started 13824
child iteration 13824: 0
parent iteration 13807: 1
^C
child done: 13824

Dart Windows Version: Dart SDK version: 2.17.3 (stable) (Wed Jun 1 11:06:41 2022 +0200) on "windows_x64"
Running this on Windows and you get this (Pressing Ctrl+C part way through):

# dart main.dart
parent started 36360
parent iteration 36360: 0
child started 24572
child iteration 24572: 0
^C
parent done: 36360
child iteration 24572: 1
child iteration 24572: 2

Pressing Ctrl+C again and you get back to the terminal prompt however the child process remains running headless.

I think this might be a duplicate or a example of "Add support for killing a process group" #22470

Relates to:

  • "IO Subscriptions keep a Dart program running" #31264
  • "Processes started with bash file flutter/bin/dart do not terminate children processes when killed" #98435
@slightfoot
Copy link
Contributor Author

slightfoot commented Jun 11, 2022

This might help:

... some research time later

This looks interesting:

DWORD creation_flags =
should probably set the CREATE_NEW_PROCESS_GROUP flag. https://docs.microsoft.com/en-us/windows/win32/procthread/process-creation-flags

Looks like

bool Process::Kill(intptr_t id, int signal) {

The kill handler doesn't call GenerateConsoleCtrlEvent when the signal is for SIGINT or SIGHUP it just always forcefully terminates.

It also looks like this should be called when the parent receives the Ctrl+C event. "If this parameter is zero, the signal is generated in all processes that share the console of the calling process."

From my understanding if the dart process calls the GenerateConsoleCtrlEvent function it will propagate to the children correctly, and you'll get cascading handlers calling ExitProcess nicely rather than forcefully terminating the child processes.

Tried this in my Ctrl+C handler and nothing. reports 1 aka TRUE from result. So perhaps the new processes are not attached to the console? perhaps this is to do with how dart spawns processes.

if (Platform.isWindows) {
  // https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent
  final kernel32 = DynamicLibrary.open('kernel32');
  final generateConsoleCtrlEvent =
      kernel32.lookupFunction<Uint32 Function(Uint32, Uint32), int Function(int, int)>(
          'GenerateConsoleCtrlEvent');
  final result = generateConsoleCtrlEvent(0, 0);
  print('Called GenerateConsoleCtrlEvent(0, 0) => $result');
}

Looking back on my notes it appears that the Job Objects API in windows is designed for this purpose. And that would be the correct solution. (Why can't windows be POSIX compliant 😢!)

https://docs.microsoft.com/en-us/windows/win32/api/jobapi2/nf-jobapi2-terminatejobobject

The GenerateConsoleCtrlEvent stuff above should probably also be done but that I think is a separate issue.

@Ali-Fadaei
Copy link

any update?

@thiagorb
Copy link

This does affect Linux as well. I created an issue on flutter repository a couple days ago, and someone replied confirming that it also happens on macOS. I also created a ticket on dart-lang repository, since the flutter one was closed.

@Isakdl
Copy link

Isakdl commented Dec 15, 2023

Other old related issues:

google/process.dart#42
#22470

@xkeyC
Copy link

xkeyC commented Feb 24, 2024

Is there an update to fix this issue? I'm using Process.start on Windows and it bothers me that the child process doesn't exit along with the parent process.

UPDATE:

On Windows, other languages have similar behavior, but there is a win32api job-objects that sets the child process to JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE which does the trick.

Since I was using flutter_rust_bridge, so I solved this problem using rust .

Some links for anyone who encounters this problem:

https://learn.microsoft.com/en-us/windows/win32/procthread/job-objects

https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-jobobject_basic_limit_information

https://learn.microsoft.com/en-us/windows/win32/api/jobapi2/nf-jobapi2-assignprocesstojobobject

@brianquinlan
Copy link
Contributor

I have a patch where I use the job approach:

--- a/runtime/bin/process_win.cc
+++ b/runtime/bin/process_win.cc
@@ -30,6 +30,7 @@ static constexpr int kWriteHandle = 1;
 int Process::global_exit_code_ = 0;
 Mutex* Process::global_exit_code_mutex_ = nullptr;
 Process::ExitHook Process::exit_hook_ = nullptr;
+static HANDLE job;

 // ProcessInfo is used to map a process id to the process handle,
 // wait handle for registered exit code event and the pipe used to
@@ -579,12 +580,15 @@ class ProcessStarter {
       return CleanupAndReturnError();
     }

     if (mode_ != kInheritStdio) {
       CloseHandle(stdin_handles_[kReadHandle]);
       CloseHandle(stdout_handles_[kWriteHandle]);
       CloseHandle(stderr_handles_[kWriteHandle]);
     }
     if (Process::ModeIsAttached(mode_)) {
+      AssignProcessToJobObject(job, process_info.hProcess);
       ProcessInfoList::AddProcess(process_info.dwProcessId,
                                   process_info.hProcess,
                                   exit_handles_[kWriteHandle]);
@@ -1119,7 +1123,12 @@ void ProcessInfoList::Cleanup() {
   ProcessInfoList::mutex_ = nullptr;
 }

+
 void Process::Init() {
+    JOBOBJECT_EXTENDED_LIMIT_INFORMATION info;
+    DWORD rc;
+    BOOL ok;
+
   ProcessInfoList::Init();

   signal_handlers = nullptr;
@@ -1132,6 +1141,21 @@ void Process::Init() {

   ASSERT(Process::global_exit_code_mutex_ == nullptr);
   Process::global_exit_code_mutex_ = new Mutex();
+
+  job = CreateJobObjectA(nullptr, nullptr);
+  ok = QueryInformationJobObject(job, JobObjectExtendedLimitInformation,
+                                   &info, sizeof(info), &rc);
+  if (!ok) {
+    printf("QueryInformationJobObject failed\n");
+  }
+
+  info.BasicLimitInformation.LimitFlags |= JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+    ok = SetInformationJobObject(job, JobObjectExtendedLimitInformation, &info,
+                                 sizeof(info));
+
+  if (!ok) {
+    printf("QueryInformationJobObject failed\n");
+  }
 }

I think that some additional flags might be needed - see Managing Processes in Jobs.

@andrewkolos
Copy link

This may be causing flutter/flutter#144222, though the issue is also producable on macOS.

ryan-andrew added a commit to ryan-andrew/android_sideloader that referenced this issue Dec 29, 2024
There is a long-standing bug in Flutter for Windows:
starting a child process will make it where the
child process never dies with us. That means adb will
remain in use by the rogue process, blocking us from
using it.

We approach this from a few angles. For normal window
closing, we can use the `AdbProcessKiller` window
listener. For terminations and crashes, we do the
following: listen for signals and kill the process
there (unreliable on Windows), and store locker files
that persist the PID of the last process that held
the adb lock.

This may only be needed for Windows. Will need to test.

See: dart-lang/sdk#49234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

8 participants