-
Notifications
You must be signed in to change notification settings - Fork 323
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
CustomHost Launch Timeout fix #265
CustomHost Launch Timeout fix #265
Conversation
// Even if TP has a timeout here, there is no way TP can abort or stop the thread/task that is hung in IDE or LUT | ||
// Even if TP can abort the API somehow, TP is essentially putting IDEs or Clients in inconsistent state without having info on | ||
// Since the IDEs own user-UI-experience here, TP will let the custom host launch as much time as IDEs define it for their users | ||
waitHandle.WaitOne(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of infinite wait, should we provide the caller an ability to pass in a timeout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what is the way for IDEs to cancel or terminate the infinite wait?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since IDE's are driving the test platform - would prefer a way for IDEs to cancel this operation instead. Just in case the process the IDE has launched has crashed before it could connect to the test platform, it would be good to have the IDEs notify the test platform so it can abort this request and process more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closure: we decided to ensure that IDE callback to launch test host (in vstestconsolewrapper) always returns a pid. This will guarantee that a stale thread will never lie around in case testhost launch fails.
@saikrishnav will probably update this PR with the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codito : We should document this in the translation layer wiki we were planning.
@saikrishnav @codito Suggestion: Seems like the documentation for this class is auto-generated. Can we improve it a bit? |
@harshjain2 definitely. #237 tracks it as well. |
testRequestManager.ResetOptions(); | ||
try | ||
{ | ||
testRequestManager.ResetOptions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need new tests for these?
…tomhosttimeoutfix
…/vstest into customhosttimeoutfix
CustomHost Launch Timeout fix