Skip to content

Commit

Permalink
Merge pull request #33485 from dotnet/merges/master-to-features/reado…
Browse files Browse the repository at this point in the history 
…nly-members

Merge master to features/readonly-members
  • Loading branch information
dotnet-automerge-bot authored Feb 19, 2019
2 parents cec9a9a + ec5be09 commit 1a26c3e
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 74 deletions.
34 changes: 34 additions & 0 deletions src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,5 +542,39 @@ public async Task IncorrectServerHashReturnsIncorrectHashResponse()
Assert.Equal(BuildResponse.ResponseType.IncorrectHash, buildResponse.Type);
}
}

[ConditionalFact(typeof(WindowsDesktopOnly))]
[WorkItem(33452, "https://github.com/dotnet/roslyn/issues/33452")]
public void QuotePipeName_Desktop()
{
var serverInfo = BuildServerConnection.GetServerProcessInfo(@"q:\tools", "name with space");
Assert.Equal(@"q:\tools\VBCSCompiler.exe", serverInfo.processFilePath);
Assert.Equal(@"q:\tools\VBCSCompiler.exe", serverInfo.toolFilePath);
Assert.Equal(@"""-pipename:name with space""", serverInfo.commandLineArguments);
}

[ConditionalFact(typeof(CoreClrOnly))]
[WorkItem(33452, "https://github.com/dotnet/roslyn/issues/33452")]
public void QuotePipeName_CoreClr()
{
var toolDir = ExecutionConditionUtil.IsWindows
? @"q:\tools"
: "/tools";
var serverInfo = BuildServerConnection.GetServerProcessInfo(toolDir, "name with space");
var vbcsFilePath = Path.Combine(toolDir, "VBCSCompiler.dll");
Assert.Equal(vbcsFilePath, serverInfo.toolFilePath);
Assert.Equal($@"exec ""{vbcsFilePath}"" ""-pipename:name with space""", serverInfo.commandLineArguments);
}

[Theory]
[InlineData(@"name with space.T.basename", "name with space", true, "basename")]
[InlineData(@"ha_ha.T.basename", @"ha""ha", true, "basename")]
[InlineData(@"jared.T.ha_ha", @"jared", true, @"ha""ha")]
[InlineData(@"jared.F.ha_ha", @"jared", false, @"ha""ha")]
[InlineData(@"jared.F.ha_ha", @"jared", false, @"ha\ha")]
public void GetPipeNameCore(string expectedName, string userName, bool isAdmin, string basePipeName)
{
Assert.Equal(expectedName, BuildServerConnection.GetPipeNameCore(userName, isAdmin, basePipeName));
}
}
}
29 changes: 26 additions & 3 deletions src/Compilers/Shared/BuildServerConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,16 @@ internal static async Task<NamedPipeClientStream> TryConnectToServerAsync(
}
}

internal static bool TryCreateServerCore(string clientDir, string pipeName)
internal static (string processFilePath, string commandLineArguments, string toolFilePath) GetServerProcessInfo(string clientDir, string pipeName)
{
var serverPathWithoutExtension = Path.Combine(clientDir, "VBCSCompiler");
var serverInfo = RuntimeHostInfo.GetProcessInfo(serverPathWithoutExtension, $"-pipename:{pipeName}");
var commandLineArgs = $@"""-pipename:{pipeName}""";
return RuntimeHostInfo.GetProcessInfo(serverPathWithoutExtension, commandLineArgs);
}

internal static bool TryCreateServerCore(string clientDir, string pipeName)
{
var serverInfo = GetServerProcessInfo(clientDir, pipeName);

if (!File.Exists(serverInfo.toolFilePath))
{
Expand Down Expand Up @@ -480,7 +486,24 @@ internal static string GetPipeNameForPathOpt(string compilerExeDirectory)
return null;
}

return $"{userName}.{(isAdmin ? 'T' : 'F')}.{basePipeName}";
return GetPipeNameCore(userName, isAdmin, basePipeName);
}

internal static string GetPipeNameCore(string userName, bool isAdmin, string basePipeName)
{
var pipeName = $"{userName}.{(isAdmin ? 'T' : 'F')}.{basePipeName}";

// The pipe name is passed between processes as a command line argument as a
// quoted value. Unfortunately we can't use ProcessStartInfo.ArgumentList as
// we still target net472 (API only available on CoreClr + netstandard). To
// make the problem approachable we remove the troublesome characters.
//
// This does mean if two users on the same machine are building simultaneously
// and the user names differ only be a " or / and a _ then there will be a
// conflict. That seems rather obscure though.
return pipeName
.Replace('"', '_')
.Replace('\\', '_');
}

/// <summary>
Expand Down
6 changes: 6 additions & 0 deletions src/Test/Utilities/Portable/Assert/ConditionalFactAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ public class ClrOnly : ExecutionCondition
public override string SkipReason => "Test not supported on Mono";
}

public class CoreClrOnly : ExecutionCondition
{
public override bool ShouldSkip => !ExecutionConditionUtil.IsCoreClr;
public override string SkipReason => "Test only supported on CoreClr";
}

public class DesktopOnly : ExecutionCondition
{
public override bool ShouldSkip => !ExecutionConditionUtil.IsDesktop;
Expand Down
121 changes: 72 additions & 49 deletions src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,65 +16,88 @@ namespace Microsoft.CodeAnalysis
internal sealed partial class Checksum : IObjectWritable, IEquatable<Checksum>
{
/// <summary>
/// The intended size of the <see cref="Sha1Hash"/> structure. Some runtime environments are known to deviate
/// from this size, so the implementation code should remain functionally correct even when the structure size
/// is greater than this value.
/// The intended size of the <see cref="HashData"/> structure.
/// </summary>
/// <seealso href="https://bugzilla.xamarin.com/show_bug.cgi?id=60298">LayoutKind.Explicit, Size = 12 ignored with 64bit alignment</seealso>
/// <seealso href="https://github.com/dotnet/roslyn/issues/23722">Checksum throws on Mono 64-bit</seealso>
private const int Sha1HashSize = 20;
private const int HashSize = 20;

public static readonly Checksum Null = new Checksum(Array.Empty<byte>());
public static readonly Checksum Null = new Checksum(default);

private Sha1Hash _checkSum;
private readonly HashData _checksum;

public unsafe Checksum(byte[] checksum)
/// <summary>
/// Create Checksum from given byte array. if byte array is bigger than
/// <see cref="HashSize"/>, it will be truncated to the size
/// </summary>
public static Checksum From(byte[] checksum)
{
if (checksum.Length == 0)
{
_checkSum = default;
return;
}
else if (checksum.Length != Sha1HashSize)
{
throw new ArgumentException($"{nameof(checksum)} must be a SHA-1 hash", nameof(checksum));
return Null;
}

fixed (byte* data = checksum)
if (checksum.Length < HashSize)
{
// Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize.
_checkSum = Sha1Hash.FromPointer((Sha1Hash*)data);
throw new ArgumentException($"checksum must be equal or bigger than the hash size: {HashSize}", nameof(checksum));
}

return FromWorker(checksum);
}

public unsafe Checksum(ImmutableArray<byte> checksum)
/// <summary>
/// Create Checksum from given byte array. if byte array is bigger than
/// <see cref="HashSize"/>, it will be truncated to the size
/// </summary>
public static Checksum From(ImmutableArray<byte> checksum)
{
if (checksum.Length == 0)
{
_checkSum = default;
return;
return Null;
}
else if (checksum.Length != Sha1HashSize)

if (checksum.Length < HashSize)
{
throw new ArgumentException($"{nameof(checksum)} must be a SHA-1 hash", nameof(checksum));
throw new ArgumentException($"{nameof(checksum)} must be equal or bigger than the hash size: {HashSize}", nameof(checksum));
}

using (var pooled = SharedPools.ByteArray.GetPooledObject())
{
var bytes = pooled.Object;
checksum.CopyTo(bytes);
checksum.CopyTo(sourceIndex: 0, bytes, destinationIndex: 0, length: HashSize);

return FromWorker(bytes);
}
}

fixed (byte* data = bytes)
{
// Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize.
_checkSum = Sha1Hash.FromPointer((Sha1Hash*)data);
}
public static Checksum FromSerialized(byte[] checksum)
{
if (checksum.Length == 0)
{
return Null;
}

if (checksum.Length != HashSize)
{
throw new ArgumentException($"{nameof(checksum)} must be equal to the hash size: {HashSize}", nameof(checksum));
}

return FromWorker(checksum);
}

private static unsafe Checksum FromWorker(byte[] checksum)
{
fixed (byte* data = checksum)
{
// Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than HashSize.
//
// ex) "https://bugzilla.xamarin.com/show_bug.cgi?id=60298" - LayoutKind.Explicit, Size = 12 ignored with 64bit alignment
// or "https://github.com/dotnet/roslyn/issues/23722" - Checksum throws on Mono 64-bit
return new Checksum(HashData.FromPointer((HashData*)data));
}
}

private Checksum(Sha1Hash hash)
private Checksum(HashData hash)
{
_checkSum = hash;
_checksum = hash;
}

public bool Equals(Checksum other)
Expand All @@ -84,24 +107,24 @@ public bool Equals(Checksum other)
return false;
}

return _checkSum == other._checkSum;
return _checksum == other._checksum;
}

public override bool Equals(object obj)
=> Equals(obj as Checksum);

public override int GetHashCode()
=> _checkSum.GetHashCode();
=> _checksum.GetHashCode();

public override unsafe string ToString()
{
var data = new byte[sizeof(Sha1Hash)];
var data = new byte[sizeof(HashData)];
fixed (byte* dataPtr = data)
{
*(Sha1Hash*)dataPtr = _checkSum;
*(HashData*)dataPtr = _checksum;
}

return Convert.ToBase64String(data, 0, Sha1HashSize);
return Convert.ToBase64String(data, 0, HashSize);
}

public static bool operator ==(Checksum left, Checksum right)
Expand All @@ -117,10 +140,10 @@ public override unsafe string ToString()
bool IObjectWritable.ShouldReuseInSerialization => true;

public void WriteTo(ObjectWriter writer)
=> _checkSum.WriteTo(writer);
=> _checksum.WriteTo(writer);

public static Checksum ReadFrom(ObjectReader reader)
=> new Checksum(Sha1Hash.ReadFrom(reader));
=> new Checksum(HashData.ReadFrom(reader));

public static string GetChecksumLogInfo(Checksum checksum)
{
Expand All @@ -133,11 +156,11 @@ public static string GetChecksumsLogInfo(IEnumerable<Checksum> checksums)
}

/// <summary>
/// This structure stores the 20-byte SHA 1 hash as an inline value rather than requiring the use of
/// This structure stores the 20-byte hash as an inline value rather than requiring the use of
/// <c>byte[]</c>.
/// </summary>
[StructLayout(LayoutKind.Explicit, Size = Sha1HashSize)]
private struct Sha1Hash : IEquatable<Sha1Hash>
[StructLayout(LayoutKind.Explicit, Size = HashSize)]
private struct HashData : IEquatable<HashData>
{
[FieldOffset(0)]
private long Data1;
Expand All @@ -148,10 +171,10 @@ private struct Sha1Hash : IEquatable<Sha1Hash>
[FieldOffset(16)]
private int Data3;

public static bool operator ==(Sha1Hash x, Sha1Hash y)
public static bool operator ==(HashData x, HashData y)
=> x.Equals(y);

public static bool operator !=(Sha1Hash x, Sha1Hash y)
public static bool operator !=(HashData x, HashData y)
=> !x.Equals(y);

public void WriteTo(ObjectWriter writer)
Expand All @@ -161,18 +184,18 @@ public void WriteTo(ObjectWriter writer)
writer.WriteInt32(Data3);
}

public static unsafe Sha1Hash FromPointer(Sha1Hash* hash)
public static unsafe HashData FromPointer(HashData* hash)
{
Sha1Hash result = default;
HashData result = default;
result.Data1 = hash->Data1;
result.Data2 = hash->Data2;
result.Data3 = hash->Data3;
return result;
}

public static Sha1Hash ReadFrom(ObjectReader reader)
public static HashData ReadFrom(ObjectReader reader)
{
Sha1Hash result = default;
HashData result = default;
result.Data1 = reader.ReadInt64();
result.Data2 = reader.ReadInt64();
result.Data3 = reader.ReadInt32();
Expand All @@ -186,9 +209,9 @@ public override int GetHashCode()
}

public override bool Equals(object obj)
=> obj is Sha1Hash other && Equals(other);
=> obj is HashData other && Equals(other);

public bool Equals(Sha1Hash other)
public bool Equals(HashData other)
{
return Data1 == other.Data1
&& Data2 == other.Data2
Expand Down
48 changes: 27 additions & 21 deletions src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,37 @@ public static Checksum Create(Stream stream)
{
using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1))
{
return ComputeChecksum(stream, hash);
}
}

private static Checksum ComputeChecksum(Stream stream, IncrementalHash hash)
{
using (var pooledBuffer = SharedPools.ByteArray.GetPooledObject())
{
stream.Seek(0, SeekOrigin.Begin);

var buffer = pooledBuffer.Object;
var bufferLength = buffer.Length;
int bytesRead;
do
using (var pooledBuffer = SharedPools.ByteArray.GetPooledObject())
{
bytesRead = stream.Read(buffer, 0, bufferLength);
if (bytesRead > 0)
stream.Seek(0, SeekOrigin.Begin);

var buffer = pooledBuffer.Object;
var bufferLength = buffer.Length;
int bytesRead;
do
{
hash.AppendData(buffer, 0, bytesRead);
bytesRead = stream.Read(buffer, 0, bufferLength);
if (bytesRead > 0)
{
hash.AppendData(buffer, 0, bytesRead);
}
}
}
while (bytesRead > 0);
while (bytesRead > 0);

var bytes = hash.GetHashAndReset();
return new Checksum(bytes);
var bytes = hash.GetHashAndReset();

// if bytes array is bigger than certain size, checksum
// will truncate it to predetermined size. for more detail,
// see the Checksum type
//
// the truncation can happen since different hash algorithm or
// same algorithm on different platform can have different hash size
// which might be bigger than the Checksum HashSize.
//
// hash algorithm used here should remain functionally correct even
// after the truncation
return Checksum.From(bytes);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ private class ChecksumJsonConverter : BaseJsonConverter<Checksum>
protected override Checksum ReadValue(JsonReader reader, JsonSerializer serializer)
{
var value = (string)reader.Value;
return value == null ? null : new Checksum(Convert.FromBase64String(value));
return value == null ? null : Checksum.FromSerialized(Convert.FromBase64String(value));
}

protected override void WriteValue(JsonWriter writer, Checksum value, JsonSerializer serializer)
Expand Down

0 comments on commit 1a26c3e

Please sign in to comment.