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

Use jsonSerializer2 for protocol version 3 #2630

Merged
merged 10 commits into from
Nov 16, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities
{
using System;
using System.IO;

using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
Expand Down Expand Up @@ -210,7 +211,29 @@ private T Deserialize<T>(JsonSerializer serializer, JToken jToken)

private JsonSerializer GetPayloadSerializer(int? version)
{
return version == 2 ? payloadSerializer2 : payloadSerializer;
if (version == null)
{
version = 1;
Sanan07 marked this conversation as resolved.
Show resolved Hide resolved
}

switch (version)
{
// 0 is used during negotiation
case 0:
case 1:
// Protocol version 3 was accidentally used with serializer v1 and not
// serializer v2, we downgrade to protocol 2 when 3 would be negotiated
// unless this is disabled by VSTEST_DISABLE_PROTOCOL_3_VERSION_DOWNGRADE
// env variable.
case 3:
return payloadSerializer;
case 2:
case 4:
return payloadSerializer2;
default:
throw new NotSupportedException($"Protocol version {version} is not supported. " +
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
"Ensure it is compatible with the latest serializer or add a new one.");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public class TestRequestSender : ITestRequestSender
// that implies host is using version 1.
private int protocolVersion = 1;

private int highestSupportedVersion = 3;
// Also check TestRequestHandler.
private int highestSupportedVersion = 4;

private TestHostConnectionInfo connectionInfo;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities
public class TestRequestHandler : ITestRequestHandler
{
private int protocolVersion = 1;
private int highestSupportedVersion = 3;

// Also check TestRequestSender.
private int highestSupportedVersion = 4;

private readonly IDataSerializer dataSerializer;
private ITestHostManagerFactory testHostManagerFactory;
Expand Down Expand Up @@ -261,7 +263,36 @@ public void OnMessageReceived(object sender, MessageReceivedEventArgs messageRec
{
case MessageType.VersionCheck:
var version = this.dataSerializer.DeserializePayload<int>(message);
this.protocolVersion = Math.Min(version, highestSupportedVersion);
// choose the highest version that we both support
var negotiatedVersion = Math.Min(version, highestSupportedVersion);
// BUT don't choose 3, because protocol version 3 has performance problems in 16.7.1-16.8. Those problems are caused
// by choosing payloadSerializer instead of payloadSerializer2 for protocol version 3.
//
// We cannot just update the code to choose the new serializer, because then that change would apply only to testhost.
// Testhost is is delivered by Microsoft.NET.Test.SDK nuget package, and can be used with an older vstest.console.
// An older vstest.console, that supports protocol version 3, would serialize its messages using payloadSerializer,
// but the fixed testhost would serialize it using payloadSerializer2, resulting in incompatible messages.
//
// Instead we must downgrade to protocol version 2 when 3 would be negotiated. Or higher when higher version
// would be negotiated.
if (negotiatedVersion != 3)
{
this.protocolVersion = negotiatedVersion;
}
else
{
var flag = Environment.GetEnvironmentVariable("VSTEST_DISABLE_PROTOCOL_3_VERSION_DOWNGRADE");
var flagIsEnabled = flag != null && flag != "0";
var dowgradeIsDisabled = flagIsEnabled;
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
if (dowgradeIsDisabled)
{
this.protocolVersion = negotiatedVersion;
}
else
{
this.protocolVersion = 2;
}
}

// Send the negotiated protocol to request sender
this.channel.Send(this.dataSerializer.SerializePayload(MessageType.VersionCheck, this.protocolVersion));
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.ObjectModel/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public static class Constants
/// <summary>
/// The default protocol version
/// </summary>
public static readonly ProtocolConfig DefaultProtocolConfig = new ProtocolConfig { Version = 3 };
public static readonly ProtocolConfig DefaultProtocolConfig = new ProtocolConfig { Version = 4 };

/// <summary>
/// The minimum protocol version that has debug support
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class VsTestConsoleRequestSender : ITranslationLayerRequestSender

private bool handShakeSuccessful = false;

private int protocolVersion = 3;
private int protocolVersion = 4;

/// <summary>
/// Use to cancel blocking tasks associated with vstest.console process
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class DesignModeClientTests

private readonly DesignModeClient designModeClient;

private readonly int protocolVersion = 3;
private readonly int protocolVersion = 4;

private readonly AutoResetEvent complateEvent;

Expand Down Expand Up @@ -131,8 +131,9 @@ public void DesignModeClientConnectShouldNotSendConnectedIfServerConnectionTimes

[TestMethod]
public void DesignModeClientDuringConnectShouldHighestCommonVersionWhenReceivedVersionIsGreaterThanSupportedVersion()
{
var verCheck = new Message { MessageType = MessageType.VersionCheck, Payload = 3 };
{

var verCheck = new Message { MessageType = MessageType.VersionCheck, Payload = 4 };
var sessionEnd = new Message { MessageType = MessageType.SessionEnd };
this.mockCommunicationManager.Setup(cm => cm.WaitForServerConnection(It.IsAny<int>())).Returns(true);
this.mockCommunicationManager.SetupSequence(cm => cm.ReceiveMessage()).Returns(verCheck).Returns(sessionEnd);
Expand Down
Loading