Sending app.stop/SIGTERM doesn't close browser

[DanTup]

This is sort of the opposite of #271. Using 2.0.0-alpha.4, when I send a stop (because the user clicked Stop on the debug toolbar), the browser remains open.

The debugger first sends app.stop, waits for the response, then it sends SIGINT then waits for the process to close. The process did close fairly quick, but it didn't close the browser.

[17:54:56 GMT+0100 (BST)] [FlutterRun] [Info] ==> [{"id":"2","method":"app.stop","params":{"appId":"JfRg0s35A+W7sj0LuNy+DA=="}}]
[17:54:56 GMT+0100 (BST)] [FlutterRun] [Info] <== [{"id":"2","result":true}]
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] TERM: Going to terminate with SIGINT...
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] TERM: Terminating related process 63686 with SIGINT...
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] TERM: Terminating main process with SIGINT...
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] TERM: Waiting for process to finish...
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] Observatory connection closed: 1006 ()
[17:54:56 GMT+0100 (BST)] [Observatory] [Info] Process exited (sigint)

[grouma]

I can't seem to reproduce this at HEAD on either Linux or Mac. Is this still an issue?

[DanTup]

Hmmm, I can repro, but only from VS Code. If I do the same thing from the terminal and paste in the app.stop command, it does not repro. The output from the logs looks exactly the same :(

I'll do some more digging!

[DanTup]

Hmmm, totally baffled. I added delays to all of the shutdown code, so nothing happens for several seconds after we call app.stop to ensure we're not interfering with any shutdown, but I still see it - when launched from VS Code the app.stop does not appear to close the browser (but it does immediately from the terminal). In fact the process terminates itself before we have chance to do anything else:

[11:20:24] ==> [{"id":"2","method":"app.stop","params":{"appId":"MBKPGwf0bCsB2T1+zGrIKw=="}}]
[11:20:24] <== [{"id":"2","result":true}]
[11:20:24] Waiting 1 second before doing any more shutdown
[11:20:24] <== [{"event":"daemon.log","params":{"log":"Stopped debug service on ws://localhost:54233\n"}}]
[11:20:24] <== [{"event":"app.log","params":{"appId":"MBKPGwf0bCsB2T1+zGrIKw==","log":"Lost connection to device."}}]
[11:20:24] <== [{"event":"app.stop","params":{"appId":"MBKPGwf0bCsB2T1+zGrIKw=="}}]
[11:20:24] Observatory connection closed: 1006 ()
[11:20:24] Process exited (sigterm)
[11:20:25] Continuing with shutdown... // Here is where we continue ensuring process is gone

If you can make a branch that has logging in the relevant code that should be running during shutdown, I can run it here and see what's going on (or I can help you get set up with VS Code if that's easier). I did try adding logging myself, but neither my prints nor daemon.send's seemed to appear in the output during shutdown.

[grouma]

@jakemac53 Is this related to our discussion today where you found that opening a DevTools instance (in another Chrome instance) keeps the daemon alive? Could VS Code have an active websocket connection to the daemon that we aren't actively closing?

[DanTup]

I think it's possible, but in the logs above it does have:

[11:20:24] Observatory connection closed: 1006 ()

This happens when the websocket we have open to the VM service closes. So it does seem like it's being closed. We do have another weird issue when launching from VS Code we haven't gotten to the bottom of yet (#319).

[grouma]

There are two actions, app.stop and daemon.shutdown.

With daemon.shutdown after the action completes, and sends a response, we shutdown down chrome:
https://github.com/dart-lang/webdev/blob/master/webdev/lib/src/command/daemon_command.dart#L79

With the app.stop action we only kill the application's tab:
https://github.com/dart-lang/webdev/blob/master/webdev/lib/src/daemon/app_domain.dart#L179

I think it's pretty safe to assume we will only ever have one application running under daemon mode so we could also run the daemon.shutdown logic after an app.stop command is called. Not sure how this is handled in Flutter or if this makes sense.

[grouma]

Looks like I was wrong. We actually do shutdown the daemon after you run app.stop because of this logic:
https://github.com/dart-lang/webdev/blob/master/webdev/lib/src/daemon/app_domain.dart#L80

This should cause the Chrome instance to close so I'm quite lost as to what is going on.

[DanTup]

I think this is the same as what's described here: #319 (comment)

I think that SIGTERM (I don't know why I said SIGINT earlier!) causes the Dart VM to terminate immediately if there are no handlers for it. I think webdev should handle SIGINT and shutdown gracefully (or if it's already shutting down, just wait for that).