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

Fix for InvalidOperationException thrown by Process.Exited #941

Merged
merged 5 commits into from Jul 18, 2017

Conversation

Projects
None yet
5 participants
@harshjain2
Member

harshjain2 commented Jul 17, 2017

Fix for InvalidOperationException thrown by Process.Exited.
Fixes #938

@harshjain2 harshjain2 requested review from codito and mayankbansal018 Jul 17, 2017

@Faizan2304

Gave a comment, please take a look. Rest look good to me 👍

{
if (proc != null && !proc.HasExited)
{
proc.Kill();

This comment has been minimized.

@Faizan2304

Faizan2304 Jul 18, 2017

Contributor

This can return Win32Exception as well if the process is terminating?
Shouldn't we catch that too?

@Faizan2304

Faizan2304 Jul 18, 2017

Contributor

This can return Win32Exception as well if the process is terminating?
Shouldn't we catch that too?

This comment has been minimized.

@harshjain2

harshjain2 Jul 18, 2017

Member

IMHO, we should bubble up Win32Exception because this exception can also be thrown if process cannot be terminated.

@harshjain2

harshjain2 Jul 18, 2017

Member

IMHO, we should bubble up Win32Exception because this exception can also be thrown if process cannot be terminated.

@codito

codito approved these changes Jul 18, 2017

@mayankbansal018

This still doesn't solve the issue, when in cleanup testhost we are disposing the test host. Agreed that no more exceptions will come, but we will lose streams, & exit codes, if disposed is called, before we complete those operations

One option is that we only dispose if host exited event is raised.

@harshjain2 harshjain2 merged commit 4642e64 into Microsoft:master Jul 18, 2017

4 checks passed

Ubuntu16.04 / Debug Build Build finished.
Details
Ubuntu16.04 / Release Build Build finished.
Details
Windows_NT / Debug Build Build finished.
Details
Windows_NT / Release Build Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment