Skip to content

Include PID in --machine output#402

Merged
DanTup merged 1 commit intoflutter:masterfrom
DanTup:add-pid
Mar 7, 2019
Merged

Include PID in --machine output#402
DanTup merged 1 commit intoflutter:masterfrom
DanTup:add-pid

Conversation

@DanTup
Copy link
Contributor

@DanTup DanTup commented Mar 7, 2019

In the editors, we have a bit of an ongoing battle with making sure we clean up processes correctly across all platforms. In many cases the process we spawn is not the real process that runs (for example on Windows running pub.bat can only be run with shellExecute, but then that runs dart.exe with the pub snapshot - so the process we spawned is at least two steps from the one we care about).

For reasons I do not entirely understand, we often see that sending a SIGINT or SIGTERM doesn't get passed down the chain correctly and we end up leaving processes hanging around. In Dart, Flutter and Flutter Test we've exposed the real PID as part of the JSON API to allow editors to also send signals to the process directly to improve the chances of terminating the process.

I can't repro leaked processes locally on macOS, however my integration tests for Dart Code are failing because I throw errors if there are Dart processes running at the end of a test run, and the DevTools ones are hanging round. The PID of the process that remains is 1 higher than the process I had a handle to (because of pub global run devtools spawning dart snapshots/pub.dart.snapshot) but the kill signals were not handled correctly.

Here, we call kill() (which sends SIGINT), then wait 5 seconds and send SIGKILL, but then at the end, there's still a process (with pid+1) hanging around.

(PROC 7152) Spawned /home/travis/build/Dart-Code/Dart-Code/with spaces/dart-sdk/bin/pub global run devtools --machine --port 0
(PROC 7152) {"method":"server.started","params":{"host":"127.0.0.1","port":38388}}
(PROC 7152) Calling kill()
(PROC 7152) Calling kill("SIGKILL")

(Test end): 7153 (1): dart: /home/travis/build/Dart-Code/Dart-Code/with spaces/dart-sdk/bin/dart /home/travis/build/Dart-Code/Dart-Code/with spaces/dart-sdk/bin/snapshots/pub.dart.snapshot global run devtools --machine --port 0

(@devoncarew @pq FYI, I'm not sure if you have all these issues in IntelliJ - if not, it'd be interested to know why not!)

@DanTup DanTup merged commit 16ecf5c into flutter:master Mar 7, 2019
@DanTup DanTup deleted the add-pid branch March 7, 2019 16:56
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.

2 participants