Skip to content

Commit 84c5476

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Merge pull request #73020 from CyrusNajmabadi/removeCancellation
2 parents cfa8c1e + c7a02cd commit 84c5476

File tree

9 files changed

+131
-62
lines changed

9 files changed

+131
-62
lines changed

‎src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ internal partial class AbstractSyntaxIndex<TIndex>
7171
if (stream != null)
7272
{
7373
using var gzipStream = new GZipStream(stream, CompressionMode.Decompress, leaveOpen: true);
74-
using var reader = ObjectReader.TryGetReader(gzipStream, cancellationToken: cancellationToken);
74+
using var reader = ObjectReader.TryGetReader(gzipStream);
7575
if (reader != null)
7676
return read(stringTable, reader, checksum);
7777
}
@@ -161,7 +161,7 @@ private async Task<bool> SaveAsync(
161161
using (var stream = SerializableBytes.CreateWritableStream())
162162
{
163163
using (var gzipStream = new GZipStream(stream, CompressionLevel.Optimal, leaveOpen: true))
164-
using (var writer = new ObjectWriter(gzipStream, leaveOpen: true, cancellationToken))
164+
using (var writer = new ObjectWriter(gzipStream, leaveOpen: true))
165165
{
166166
WriteTo(writer);
167167
gzipStream.Flush();

‎src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private static async Task<SymbolTreeInfo> LoadOrCreateAsync(
6565

6666
using (var stream = SerializableBytes.CreateWritableStream())
6767
{
68-
using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
68+
using (var writer = new ObjectWriter(stream, leaveOpen: true))
6969
{
7070
result.WriteTo(writer);
7171
}
@@ -98,7 +98,7 @@ private static async Task<SymbolTreeInfo> LoadOrCreateAsync(
9898

9999
// If the checksum doesn't need to match, then we can pass in 'null' here allowing any result to be found.
100100
using var stream = await storage.ReadStreamAsync(key, checksumMustMatch ? checksum : null, cancellationToken).ConfigureAwait(false);
101-
using var reader = ObjectReader.TryGetReader(stream, cancellationToken: cancellationToken);
101+
using var reader = ObjectReader.TryGetReader(stream);
102102

103103
// We have some previously persisted data. Attempt to read it back.
104104
// If we're able to, and the version of the persisted data matches

‎src/Workspaces/Core/Portable/Serialization/SerializerService_Reference.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public static Checksum CreateChecksum(AnalyzerReference reference, CancellationT
4444

4545
using var stream = SerializableBytes.CreateWritableStream();
4646

47-
using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
47+
using (var writer = new ObjectWriter(stream, leaveOpen: true))
4848
{
4949
switch (reference)
5050
{
@@ -143,7 +143,7 @@ private static Checksum CreatePortableExecutableReferenceChecksum(PortableExecut
143143
{
144144
using var stream = SerializableBytes.CreateWritableStream();
145145

146-
using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
146+
using (var writer = new ObjectWriter(stream, leaveOpen: true))
147147
{
148148
WritePortableExecutableReferencePropertiesTo(reference, writer, cancellationToken);
149149
WriteMvidsTo(TryGetMetadata(reference), writer, cancellationToken);

‎src/Workspaces/CoreTest/ObjectSerializationTests.cs‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,78 @@ public void Encodings(Encoding original)
668668
EncodingTestHelpers.AssertEncodingsEqual(original, deserialized);
669669
}
670670

671+
[Fact]
672+
public void TestMultipleAssetWritingAndReader()
673+
{
674+
using var stream = new MemoryStream();
675+
676+
const string GooString = "Goo";
677+
const string BarString = "Bar";
678+
var largeString = new string('a', 1024);
679+
680+
// Write out some initial bytes, to demonstrate the reader not throwing, even if we don't have the right
681+
// validation bytes at the start.
682+
stream.WriteByte(1);
683+
stream.WriteByte(2);
684+
685+
using (var writer = new ObjectWriter(stream, leaveOpen: true, writeValidationBytes: false))
686+
{
687+
writer.WriteValidationBytes();
688+
writer.WriteString(GooString);
689+
writer.WriteString("Bar");
690+
writer.WriteString(largeString);
691+
692+
// Random data, not going through the writer.
693+
stream.WriteByte(3);
694+
stream.WriteByte(4);
695+
696+
// We should be able to write out a new object, using strings we've already seen.
697+
writer.WriteValidationBytes();
698+
writer.WriteString(largeString);
699+
writer.WriteString("Bar");
700+
writer.WriteString(GooString);
701+
}
702+
703+
stream.Position = 0;
704+
705+
using var reader = ObjectReader.GetReader(stream, leaveOpen: true, checkValidationBytes: false);
706+
707+
Assert.Equal(1, reader.ReadByte());
708+
Assert.Equal(2, reader.ReadByte());
709+
710+
reader.CheckValidationBytes();
711+
712+
var string1 = reader.ReadString();
713+
var string2 = reader.ReadString();
714+
var string3 = reader.ReadString();
715+
Assert.Equal(GooString, string1);
716+
Assert.Equal(BarString, string2);
717+
Assert.Equal(largeString, string3);
718+
Assert.NotSame(GooString, string1);
719+
Assert.NotSame(BarString, string2);
720+
Assert.NotSame(largeString, string3);
721+
722+
Assert.Equal(3, stream.ReadByte());
723+
Assert.Equal(4, stream.ReadByte());
724+
725+
reader.CheckValidationBytes();
726+
var string4 = reader.ReadString();
727+
var string5 = reader.ReadString();
728+
var string6 = reader.ReadString();
729+
Assert.Equal(largeString, string4);
730+
Assert.Equal(BarString, string5);
731+
Assert.Equal(GooString, string6);
732+
Assert.NotSame(largeString, string4);
733+
Assert.NotSame(BarString, string5);
734+
Assert.NotSame(GooString, string6);
735+
736+
// These should be references to the same strings in the format string and should return the values already
737+
// returned.
738+
Assert.Same(string1, string6);
739+
Assert.Same(string2, string5);
740+
Assert.Same(string3, string4);
741+
}
742+
671743
// keep these around for analyzing perf issues
672744
#if false
673745
[Fact]

‎src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ public ValueTask GetAssetsAsync<T, TArg>(
2828
using var stream = new MemoryStream();
2929
using var context = new SolutionReplicationContext();
3030

31-
using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
31+
using (var writer = new ObjectWriter(stream, leaveOpen: true))
3232
{
3333
serializerService.Serialize(data, writer, context, cancellationToken);
3434
}
3535

3636
stream.Position = 0;
37-
using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken);
37+
using var reader = ObjectReader.GetReader(stream, leaveOpen: true);
3838
var asset = deserializerService.Deserialize(data.GetWellKnownSynchronizationKind(), reader, cancellationToken);
3939
Contract.ThrowIfNull(asset);
4040
callback(checksum, (T)asset, arg);

‎src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static async ValueTask WriteDataAsync(
2626
ReadOnlyMemory<Checksum> checksums,
2727
CancellationToken cancellationToken)
2828
{
29-
using var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken);
29+
using var writer = new ObjectWriter(stream, leaveOpen: true);
3030

3131
// This information is not actually needed on the receiving end. However, we still send it so that the
3232
// receiver can assert that both sides are talking about the same solution snapshot and no weird invariant
@@ -41,6 +41,7 @@ public static async ValueTask WriteDataAsync(
4141

4242
foreach (var (checksum, asset) in assetMap)
4343
{
44+
cancellationToken.ThrowIfCancellationRequested();
4445
Contract.ThrowIfNull(asset);
4546

4647
var kind = asset.GetWellKnownSynchronizationKind();
@@ -79,7 +80,7 @@ static async ValueTask ReadDataSuppressedFlowAsync(
7980
public static void ReadData<T, TArg>(
8081
Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, Action<Checksum, T, TArg> callback, TArg arg, CancellationToken cancellationToken)
8182
{
82-
using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken);
83+
using var reader = ObjectReader.GetReader(stream, leaveOpen: true);
8384

8485
// Ensure that no invariants were broken and that both sides of the communication channel are talking about
8586
// the same pinned solution.
@@ -88,6 +89,7 @@ public static void ReadData<T, TArg>(
8889

8990
for (int i = 0, n = objectCount; i < n; i++)
9091
{
92+
cancellationToken.ThrowIfCancellationRequested();
9193
var checksum = Checksum.ReadFrom(reader);
9294
var kind = (WellKnownSynchronizationKind)reader.ReadByte();
9395

‎src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.Caching.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ private static async Task CacheClassificationsAsync(
156156
}
157157

158158
using var stream = SerializableBytes.CreateWritableStream();
159-
using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
159+
using (var writer = new ObjectWriter(stream, leaveOpen: true))
160160
{
161161
WriteTo(classifiedSpans, writer);
162162
}
@@ -286,7 +286,7 @@ private async Task<ImmutableArray<ClassifiedSpan>> TryReadCachedSemanticClassifi
286286

287287
var persistenceName = GetPersistenceName(type);
288288
using var stream = await storage.ReadStreamAsync(documentKey, persistenceName, checksum, cancellationToken).ConfigureAwait(false);
289-
using var reader = ObjectReader.TryGetReader(stream, cancellationToken: cancellationToken);
289+
using var reader = ObjectReader.TryGetReader(stream);
290290
if (reader == null)
291291
return default;
292292

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Serialization/ObjectReader.cs‎

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using System.IO;
88
using System.Runtime.ExceptionServices;
99
using System.Text;
10-
using System.Threading;
1110
using Microsoft.CodeAnalysis;
1211

1312
namespace Roslyn.Utilities;
@@ -28,7 +27,6 @@ internal sealed partial class ObjectReader : IDisposable
2827
internal const byte VersionByte2 = 0b00001101;
2928

3029
private readonly BinaryReader _reader;
31-
private readonly CancellationToken _cancellationToken;
3230

3331
/// <summary>
3432
/// Map of reference id's to deserialized strings.
@@ -40,31 +38,22 @@ internal sealed partial class ObjectReader : IDisposable
4038
/// </summary>
4139
/// <param name="stream">The stream to read objects from.</param>
4240
/// <param name="leaveOpen">True to leave the <paramref name="stream"/> open after the <see cref="ObjectWriter"/> is disposed.</param>
43-
/// <param name="cancellationToken"></param>
44-
private ObjectReader(
45-
Stream stream,
46-
bool leaveOpen,
47-
CancellationToken cancellationToken)
41+
private ObjectReader(Stream stream, bool leaveOpen)
4842
{
4943
// String serialization assumes both reader and writer to be of the same endianness.
5044
// It can be adjusted for BigEndian if needed.
5145
Debug.Assert(BitConverter.IsLittleEndian);
5246

5347
_reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen);
5448
_stringReferenceMap = ReaderReferenceMap.Create();
55-
56-
_cancellationToken = cancellationToken;
5749
}
5850

5951
/// <summary>
6052
/// Attempts to create a <see cref="ObjectReader"/> from the provided <paramref name="stream"/>.
6153
/// If the <paramref name="stream"/> does not start with a valid header, then <see langword="null"/> will
6254
/// be returned.
6355
/// </summary>
64-
public static ObjectReader? TryGetReader(
65-
Stream? stream,
66-
bool leaveOpen = false,
67-
CancellationToken cancellationToken = default)
56+
public static ObjectReader? TryGetReader(Stream? stream, bool leaveOpen = false)
6857
{
6958
if (stream == null)
7059
{
@@ -91,43 +80,42 @@ private ObjectReader(
9180
#endif
9281
}
9382

94-
return new ObjectReader(stream, leaveOpen, cancellationToken);
83+
return new ObjectReader(stream, leaveOpen);
9584
}
9685

97-
/// <summary>
98-
/// Creates an <see cref="ObjectReader"/> from the provided <paramref name="stream"/>.
99-
/// Unlike <see cref="TryGetReader(Stream, bool, CancellationToken)"/>, it requires the version
100-
/// of the data in the stream to exactly match the current format version.
101-
/// Should only be used to read data written by the same version of Roslyn.
86+
/// <summary>
87+
/// Creates an <see cref="ObjectReader"/> from the provided <paramref name="stream"/>. Unlike <see
88+
/// cref="TryGetReader(Stream, bool)"/>, it requires the version of the data in the stream to
89+
/// exactly match the current format version. Should only be used to read data written by the same version of
90+
/// Roslyn.
10291
/// </summary>
103-
public static ObjectReader GetReader(
104-
Stream stream,
105-
bool leaveOpen,
106-
CancellationToken cancellationToken)
92+
public static ObjectReader GetReader(Stream stream, bool leaveOpen)
93+
=> GetReader(stream, leaveOpen, checkValidationBytes: true);
94+
95+
/// <summary>
96+
/// <inheritdoc cref="GetReader(Stream, bool)"/>
97+
/// <param name="checkValidationBytes">Whether or not the validation bytes (see <see
98+
/// cref="ObjectWriter.WriteValidationBytes"/> should be checked immediately at the stream's current
99+
/// position.</param>
100+
/// </summary>
101+
public static ObjectReader GetReader(Stream stream, bool leaveOpen, bool checkValidationBytes)
107102
{
108-
var b = stream.ReadByte();
109-
if (b == -1)
110-
{
111-
throw new EndOfStreamException();
112-
}
103+
var reader = new ObjectReader(stream, leaveOpen);
104+
if (checkValidationBytes)
105+
reader.CheckValidationBytes();
106+
107+
return reader;
108+
}
113109

110+
public void CheckValidationBytes()
111+
{
112+
var b = this.ReadByte();
114113
if (b != VersionByte1)
115-
{
116114
throw ExceptionUtilities.UnexpectedValue(b);
117-
}
118-
119-
b = stream.ReadByte();
120-
if (b == -1)
121-
{
122-
throw new EndOfStreamException();
123-
}
124115

116+
b = this.ReadByte();
125117
if (b != VersionByte2)
126-
{
127118
throw ExceptionUtilities.UnexpectedValue(b);
128-
}
129-
130-
return new ObjectReader(stream, leaveOpen, cancellationToken);
131119
}
132120

133121
public void Dispose()

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Serialization/ObjectWriter.cs‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Reflection;
99
using System.Runtime.InteropServices;
1010
using System.Text;
11-
using System.Threading;
1211
using Microsoft.CodeAnalysis;
1312
using Microsoft.CodeAnalysis.PooledObjects;
1413
using EncodingExtensions = Microsoft.CodeAnalysis.EncodingExtensions;
@@ -53,7 +52,6 @@ private static class BufferPool<T>
5352
public const byte Byte4Marker = 2 << 6;
5453

5554
private readonly BinaryWriter _writer;
56-
private readonly CancellationToken _cancellationToken;
5755

5856
/// <summary>
5957
/// Map of serialized string reference ids. The string-reference-map uses value-equality for greater cache hits
@@ -81,24 +79,33 @@ private static class BufferPool<T>
8179
/// </summary>
8280
/// <param name="stream">The stream to write to.</param>
8381
/// <param name="leaveOpen">True to leave the <paramref name="stream"/> open after the <see cref="ObjectWriter"/> is disposed.</param>
84-
/// <param name="cancellationToken">Cancellation token.</param>
85-
public ObjectWriter(
86-
Stream stream,
87-
bool leaveOpen = false,
88-
CancellationToken cancellationToken = default)
82+
public ObjectWriter(Stream stream, bool leaveOpen = false)
83+
: this(stream, leaveOpen, writeValidationBytes: true)
84+
{
85+
}
86+
87+
/// <inheritdoc cref="ObjectWriter(Stream, bool)"/>
88+
/// <param name="writeValidationBytes">Whether or not the validation bytes (see <see cref="WriteValidationBytes"/>)
89+
/// should be immediately written into the stream.</param>
90+
public ObjectWriter(Stream stream, bool leaveOpen, bool writeValidationBytes)
8991
{
9092
// String serialization assumes both reader and writer to be of the same endianness.
9193
// It can be adjusted for BigEndian if needed.
9294
Debug.Assert(BitConverter.IsLittleEndian);
9395

9496
_writer = new BinaryWriter(stream, Encoding.UTF8, leaveOpen);
9597
_stringReferenceMap = new WriterReferenceMap();
96-
_cancellationToken = cancellationToken;
9798

98-
WriteVersion();
99+
if (writeValidationBytes)
100+
WriteValidationBytes();
99101
}
100102

101-
private void WriteVersion()
103+
/// <summary>
104+
/// Writes out a special sequence of bytes indicating that the stream is a serialized object stream. Used by the
105+
/// <see cref="ObjectReader"/> to be able to easily detect if it is being improperly used, or if the stream is
106+
/// corrupt.
107+
/// </summary>
108+
public void WriteValidationBytes()
102109
{
103110
WriteByte(ObjectReader.VersionByte1);
104111
WriteByte(ObjectReader.VersionByte2);

0 commit comments

Comments
 (0)