Skip to content

Commit

Permalink
Use jsonSerializer2 for protocol version 3 (#2630)
Browse files Browse the repository at this point in the history
* Use jsonSerializer2 for protocol version 3

* Use serializer tests for protocol 2 and 3, and throw on new protocol version

* Add version 0 for negotiation

* Downgrade to Protocol2 when used with older vstest.console

* Update defaults to protocol 4.
  • Loading branch information
nohwnd committed Nov 16, 2020
1 parent 6aa297d commit a7046e3
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 51 deletions.
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;
}

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. " +
"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;
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

0 comments on commit a7046e3

Please sign in to comment.