dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

Child processes remain after parent termination on Windows #49234

Open slightfoot opened 2 years ago

slightfoot commented 2 years ago

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:

slightfoot commented 2 years ago

This might help:

... some research time later

This looks interesting: https://github.com/dart-lang/sdk/blob/de45656da1885cbc761fb45e8f63cef0ae6ba52a/runtime/bin/process_win.cc#L552 should probably set the CREATE_NEW_PROCESS_GROUP flag. https://docs.microsoft.com/en-us/windows/win32/procthread/process-creation-flags

Looks like https://github.com/dart-lang/sdk/blob/de45656da1885cbc761fb45e8f63cef0ae6ba52a/runtime/bin/process_win.cc#L904

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 commented 1 year ago

any update?

thiagorb commented 11 months ago

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 commented 9 months ago

Other old related issues:

https://github.com/google/process.dart/issues/42 https://github.com/dart-lang/sdk/issues/22470

xkeyC commented 6 months ago

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 commented 6 months ago

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 commented 4 months ago

This may be causing https://github.com/flutter/flutter/issues/144222, though the issue is also producable on macOS.