Skip to content

Suspicious cancellation logic in MTP #6099

@Youssef1313

Description

@Youssef1313

protected static async Task ExecuteRequestAsync(ProxyOutputDevice outputDevice, ITestSessionContext testSessionInfo,
ServiceProvider serviceProvider, BaseMessageBus baseMessageBus, ITestFramework testFramework, TestHost.ClientInfo client)
{
CancellationToken testSessionCancellationToken = serviceProvider.GetTestSessionContext().CancellationToken;
await DisplayBeforeSessionStartAsync(outputDevice, testSessionInfo, testSessionCancellationToken).ConfigureAwait(false);
try
{
await NotifyTestSessionStartAsync(testSessionInfo.SessionId, baseMessageBus, serviceProvider, testSessionCancellationToken).ConfigureAwait(false);
await serviceProvider.GetTestAdapterInvoker().ExecuteAsync(testFramework, client, testSessionCancellationToken).ConfigureAwait(false);
await NotifyTestSessionEndAsync(testSessionInfo.SessionId, baseMessageBus, serviceProvider, testSessionCancellationToken).ConfigureAwait(false);
}
catch (OperationCanceledException) when (testSessionCancellationToken.IsCancellationRequested)
{
// Do nothing we're canceled
}
// We keep the display after session out of the OperationCanceledException catch because we want to notify the IPlatformOutputDevice
// also in case of cancellation. Most likely it needs to notify users that the session was canceled.
await DisplayAfterSessionEndRunAsync(outputDevice, testSessionInfo, testSessionCancellationToken).ConfigureAwait(false);
}

We always call DisplayAfterSessionEndRunAsync at the end, regardless of whether or not we are canceled. But then we pass testSessionCancellationToken. That feels suspicious to me.

The comment says:

        // We keep the display after session out of the OperationCanceledException catch because we want to notify the IPlatformOutputDevice
        // also in case of cancellation. Most likely it needs to notify users that the session was canceled.

But in that case shouldn't we pass CancellationToken.None to explicitly state that we want the work to be done even in case of cancellation?

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: MTPBelongs to the Microsoft.Testing.Platform core library

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions