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

Add a version header to Object-Streams. #15794

Merged
merged 9 commits into from Dec 10, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs
Expand Up @@ -204,8 +204,14 @@ public static SyntaxNode DeserializeFrom(Stream stream, CancellationToken cancel
throw new InvalidOperationException(CodeAnalysisResources.TheStreamCannotBeReadFrom);
}

using (var reader = new StreamObjectReader(stream, knownObjects: GetDeserializationObjectData(), binder: s_defaultBinder, cancellationToken: cancellationToken))

using (var reader = StreamObjectReader.TryGetReader(stream, knownObjects: GetDeserializationObjectData(), binder: s_defaultBinder, cancellationToken: cancellationToken))
{
if (reader == null)
{
throw new ArgumentException(CodeAnalysisResources.Stream_contains_invalid_data, nameof(stream));
}

var root = (Syntax.InternalSyntax.CSharpSyntaxNode)reader.ReadValue();
return root.CreateRed();
}
Expand Down
19 changes: 16 additions & 3 deletions src/Compilers/Core/CodeAnalysisTest/ObjectSerializationTests.cs
Expand Up @@ -12,6 +12,19 @@ namespace Microsoft.CodeAnalysis.UnitTests
{
public sealed class ObjectSerializationTests
{
[Fact]
private void TestInvalidStreamVersion()
{
var stream = new MemoryStream();
stream.WriteByte(0);
stream.WriteByte(0);

stream.Position = 0;

var reader = StreamObjectReader.TryGetReader(stream);
Assert.Null(reader);
}

private void RoundTrip(Action<ObjectWriter> writeAction, Action<ObjectReader> readAction, bool recursive)
{
var stream = new MemoryStream();
Expand All @@ -25,7 +38,7 @@ private void RoundTrip(Action<ObjectWriter> writeAction, Action<ObjectReader> re
Assert.Equal(recursive, StreamObjectReader.IsRecursive(stream));

stream.Position = 0;
using (var reader = new StreamObjectReader(stream, binder: binder))
using (var reader = StreamObjectReader.TryGetReader(stream, binder: binder))
{
readAction(reader);
}
Expand All @@ -50,7 +63,7 @@ private T RoundTrip<T>(T value, Action<ObjectWriter, T> writeAction, Func<Object
Assert.Equal(recursive, StreamObjectReader.IsRecursive(stream));

stream.Position = 0;
using (var reader = new StreamObjectReader(stream, binder: binder))
using (var reader = StreamObjectReader.TryGetReader(stream, binder: binder))
{
return (T)readAction(reader);
}
Expand Down Expand Up @@ -978,7 +991,7 @@ public void TestObjectMapLimits()
writer.Dispose();

stream.Position = 0;
using (var reader = new StreamObjectReader(stream, binder: binder))
using (var reader = StreamObjectReader.TryGetReader(stream, binder: binder))
{
for (int pass = 0; pass < 2; pass++)
{
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/Core/Portable/CodeAnalysisResources.resx
Expand Up @@ -588,4 +588,7 @@
<data name="Deserialization_reader_for_0_read_incorrect_number_of_values" xml:space="preserve">
<value>Deserialization reader for '{0}' read incorrect number of values.</value>
</data>
<data name="Stream_contains_invalid_data" xml:space="preserve">
<value>Stream contains invalid data</value>
</data>
</root>
40 changes: 36 additions & 4 deletions src/Compilers/Core/Portable/Serialization/StreamObjectReader.cs
Expand Up @@ -27,6 +27,14 @@ namespace Roslyn.Utilities
/// </summary>
internal sealed partial class StreamObjectReader : ObjectReader, IDisposable
{
/// <summary>
/// We start the version at something reasonably random. That way an older file, with
/// some random start-bytes, has little chance of matching our version. When incrementing
/// this version, just change VersionByte2.
/// </summary>
internal const byte VersionByte1 = 0b10101010;
internal const byte VersionByte2 = 0b00000001;
Copy link
Contributor

Choose a reason for hiding this comment

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

Binary literals FTW!


private readonly BinaryReader _reader;
private readonly ObjectBinder _binder;
private readonly bool _recursive;
Expand Down Expand Up @@ -67,11 +75,11 @@ internal sealed partial class StreamObjectReader : ObjectReader, IDisposable
/// <param name="knownObjects">An optional list of objects assumed known by the corresponding <see cref="StreamObjectWriter"/>.</param>
/// <param name="binder">A binder that provides object and type decoding.</param>
/// <param name="cancellationToken"></param>
public StreamObjectReader(
private StreamObjectReader(
Stream stream,
ObjectData knownObjects = null,
ObjectBinder binder = null,
CancellationToken cancellationToken = default(CancellationToken))
ObjectData knownObjects,
ObjectBinder binder,
CancellationToken cancellationToken)
{
// String serialization assumes both reader and writer to be of the same endianness.
// It can be adjusted for BigEndian if needed.
Expand All @@ -93,6 +101,30 @@ internal sealed partial class StreamObjectReader : ObjectReader, IDisposable
}
}

/// <summary>
/// Attempts to create a <see cref="StreamObjectReader"/> from the provided <paramref name="stream"/>.
/// If the <paramref name="stream"/> does not start with a valid header, then <code>null</code> will
/// be returned.
/// </summary>
public static StreamObjectReader TryGetReader(
Stream stream,
ObjectData knownObjects = null,
ObjectBinder binder = null,
CancellationToken cancellationToken = default(CancellationToken))
{
if (stream == null)
{
return null;
}

if (stream.ReadByte() != VersionByte1 && stream.ReadByte() != VersionByte2)
{
return null;
}

return new StreamObjectReader(stream, knownObjects, binder, cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could put the read of the format (recursive, etc) byte here too and have it also return null if it doesn't match one of the known types.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could. But that indicates a true bug. That means the format changed and the version wasn't updated. That's actually a bug, and i want an exceptoin in that case.

}

internal static bool IsRecursive(Stream stream)
{
var recursionKind = (EncodingKind)stream.ReadByte();
Expand Down
Expand Up @@ -82,6 +82,8 @@ internal sealed partial class StreamObjectWriter : ObjectWriter, IDisposable
_recursive = recursive;
_cancellationToken = cancellationToken;

WriteVersion();

if (_recursive)
{
_writer.Write((byte)EncodingKind.Recursive);
Expand All @@ -95,6 +97,12 @@ internal sealed partial class StreamObjectWriter : ObjectWriter, IDisposable
}
}

private void WriteVersion()
{
_writer.Write(StreamObjectReader.VersionByte1);
_writer.Write(StreamObjectReader.VersionByte2);
}

public void Dispose()
{
_referenceMap.Dispose();
Expand Down
Expand Up @@ -140,7 +140,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Throw New InvalidOperationException(CodeAnalysisResources.TheStreamCannotBeReadFrom)
End If

Using reader = New StreamObjectReader(stream, knownObjects:=GetDeserializationObjectData(), binder:=s_defaultBinder, cancellationToken:=cancellationToken)
Using reader = StreamObjectReader.TryGetReader(stream, knownObjects:=GetDeserializationObjectData(), binder:=s_defaultBinder, cancellationToken:=cancellationToken)
If reader Is Nothing Then
Throw New ArgumentException(CodeAnalysisResources.Stream_contains_invalid_data, NameOf(stream))
End If

Return DirectCast(reader.ReadValue(), InternalSyntax.VisualBasicSyntaxNode).CreateRed(Nothing, 0)
End Using
End Function
Expand Down
@@ -1,7 +1,5 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Threading;
Expand Down Expand Up @@ -32,33 +30,25 @@ protected override int GetCount(Data data)

protected override Data TryGetExistingData(Stream stream, Document value, CancellationToken cancellationToken)
{
var list = SharedPools.Default<List<TodoItem>>().AllocateAndClear();
try
using (var reader = StreamObjectReader.TryGetReader(stream))
{
using (var reader = new StreamObjectReader(stream))
if (reader != null)
{
var format = reader.ReadString();
if (!string.Equals(format, FormatVersion))
if (string.Equals(format, FormatVersion))
{
return null;
}

var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);
var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);

AppendItems(reader, value, list, cancellationToken);
var list = ArrayBuilder<TodoItem>.GetInstance();
AppendItems(reader, value, list, cancellationToken);

return new Data(textVersion, dataVersion, list.ToImmutableArray<TodoItem>());
return new Data(textVersion, dataVersion, list.ToImmutableAndFree());
}
}
}
catch (Exception)
{
return null;
}
finally
{
SharedPools.Default<List<TodoItem>>().ClearAndFree(list);
}

return null;
}

protected override void WriteTo(Stream stream, Data data, CancellationToken cancellationToken)
Expand Down Expand Up @@ -104,7 +94,7 @@ public ImmutableArray<TodoItem> GetItems_TestingOnly(DocumentId documentId)
return ImmutableArray<TodoItem>.Empty;
}

private void AppendItems(ObjectReader reader, Document document, List<TodoItem> list, CancellationToken cancellationToken)
private void AppendItems(ObjectReader reader, Document document, ArrayBuilder<TodoItem> list, CancellationToken cancellationToken)
{
var count = reader.ReadInt32();
for (var i = 0; i < count; i++)
Expand All @@ -130,4 +120,4 @@ private void AppendItems(ObjectReader reader, Document document, List<TodoItem>
}
}
}
}
}
@@ -1,10 +1,12 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.SolutionCrawler.State
Expand Down Expand Up @@ -54,16 +56,22 @@ public async Task<TData> TryGetExistingDataAsync(TValue value, CancellationToken
var solution = GetSolution(value);
var persistService = solution.Workspace.Services.GetService<IPersistentStorageService>();

using (var storage = persistService.GetStorage(solution))
using (var stream = await ReadStreamAsync(storage, value, cancellationToken).ConfigureAwait(false))
try
{
if (stream == null)
using (var storage = persistService.GetStorage(solution))
using (var stream = await ReadStreamAsync(storage, value, cancellationToken).ConfigureAwait(false))
{
return default(TData);
if (stream != null)
{
return TryGetExistingData(stream, value, cancellationToken);
}
}

return TryGetExistingData(stream, value, cancellationToken);
}
catch (Exception e) when (IOUtilities.IsNormalIOException(e))
{
}

return default(TData);
}

public async Task PersistAsync(TValue value, TData data, CancellationToken cancellationToken)
Expand Down
Expand Up @@ -31,27 +31,23 @@ protected override int GetCount(Data data)

protected override Data TryGetExistingData(Stream stream, Document value, CancellationToken cancellationToken)
{
try
using (var reader = StreamObjectReader.TryGetReader(stream))
{
using (var reader = new StreamObjectReader(stream))
if (reader != null)
{
var format = reader.ReadString();
if (!string.Equals(format, FormatVersion, StringComparison.InvariantCulture))
if (string.Equals(format, FormatVersion, StringComparison.InvariantCulture))
{
return null;
}

var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);
var designerAttributeArgument = reader.ReadString();
var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);
var designerAttributeArgument = reader.ReadString();

return new Data(textVersion, dataVersion, designerAttributeArgument);
return new Data(textVersion, dataVersion, designerAttributeArgument);
}
}
}
catch (Exception)
{
return null;
}

return null;
}

protected override void WriteTo(Stream stream, Data data, CancellationToken cancellationToken)
Expand Down
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -139,8 +140,11 @@ private CompilationWithAnalyzers CreateAnalyzerDriver(CompilationWithAnalyzers a
// handling of cancellation and exception
var version = await DiagnosticIncrementalAnalyzer.GetDiagnosticVersionAsync(project, cancellationToken).ConfigureAwait(false);

using (var reader = new StreamObjectReader(stream))
using (var reader = StreamObjectReader.TryGetReader(stream))
{
Debug.Assert(reader != null,
@"We only ge a reader for data transmitted between live processes.
Copy link
Contributor

Choose a reason for hiding this comment

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

get?

This data should always be correct as we're never persisting the data between sessions.");
return DiagnosticResultSerializer.Deserialize(reader, analyzerMap, project, version, cancellationToken);
}
}
Expand Down