Skip to content
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 a Cancellation timing issue #1397

Closed

Conversation

AbhitejJohn
Copy link
Contributor

Fix for a timing issue where a cancel request before test run request has been started is being ignored.

We have the right plumbing in place but just seem to have missed initializing a var. Put that in place.
All interface changes are internal only.

Related issue

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/559064

From testhost diag logs, this is the stack trace:
TpTrace Error: 0 : 17096, 8, 2018/01/30, 15:50:44.560, 1284210642450, testhost.x86.exe, LengthPrefixCommunicationChannel: MessageReceived: Exception occurred while calling handler of type Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TestRequestHandler for MessageReceivedEventArgs: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution.ExecutionManager.Cancel()
at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TestRequestHandler.OnMessageReceived(Object sender, MessageReceivedEventArgs messageReceivedArgs)
--- End of inner exception stack trace ---
at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
at System.Delegate.DynamicInvokeImpl(Object[] args)
at Microsoft.VisualStudio.TestPlatform.Utilities.MulticastDelegateUtilities.SafeInvoke(Delegate delegates, Object sender, EventArgs args, String traceDisplayName)

… has been started is being ignored.

We have the right plumbing in place but just seem to have missed initializing a var. Put that in place.
All interface changes are internal only.
@AbhitejJohn
Copy link
Contributor Author

This might be targeted to the wrong branch. Do let me know which branch I should target to for 15.6.

@@ -168,7 +168,8 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
/// <summary>
/// Cancels the test run.
/// </summary>
public virtual void Cancel()
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
public virtual void Cancel(ITestRunEventsHandler eventHandler)
{
// Cancel fast, try to stop testhost deployment/launch
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on our assumption everywhere else, that is Cancel comes before StartTestRun.
If Cancel comes, it will result in a HandleTestRunComplete, where we have a baseTestRunEventsHandler which only gets set if Start was called, else it could also potentially cause null reference.

@singhsarab
Copy link
Contributor

@AbhitejJohn Does this have all the changes? Please update and push this one in.

@singhsarab
Copy link
Contributor

Created another for port #1436

@singhsarab singhsarab closed this Feb 19, 2018
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