Skip to content

Conversation

@outermeasure
Copy link
Contributor

@outermeasure outermeasure commented Feb 12, 2021

Calling this.process.dispose(); before this.process.shutdown(true); sets a flag _isDisposed to true.
This causes the _kill() procedure to exit early essentially making this.process.shutdown(true) a no op.

It's necessary to await on the shutdown because _kill() is an async function. Otherwise _isDisposed will be set to true before actually killing the process regardless of the call order of dispose and shutdown.

Impact: Closing the session in the browser should now kill long running processes such as filesystem watchers or services started in the terminal.

@code-asher code-asher self-requested a review February 16, 2021 23:44
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Thank you for the fix and the detailed explanation!

@code-asher code-asher merged commit 271c260 into coder:master Feb 22, 2021
@outermeasure outermeasure deleted the master branch February 22, 2021 23:08
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