Skip to content

Commit 6427a2b

Browse files
Prevent stack overflow by trampolining to another thread.
1 parent ff39dda commit 6427a2b

File tree

3 files changed

+88
-198
lines changed

3 files changed

+88
-198
lines changed

src/Compilers/Core/CodeAnalysisTest/ObjectSerializationTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,11 +1114,11 @@ public void TestWideObjectGraph()
11141114
}
11151115

11161116
[Fact]
1117-
public void TestDeepObjectGraph_RecursiveFails()
1117+
public void TestDeepObjectGraph_RecursiveSucceeds()
11181118
{
11191119
int id = 0;
11201120
var graph = ConstructGraph(ref id, 1, 1000);
1121-
Assert.Throws<ObjectWriter.RecursionDepthExceeded>(() => TestRoundTripValue(graph));
1121+
TestRoundTripValue(graph);
11221122
}
11231123

11241124
[Fact]

src/Compilers/Core/Portable/Serialization/StreamObjectReader.cs

Lines changed: 41 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Runtime.CompilerServices;
99
using System.Text;
1010
using System.Threading;
11+
using System.Threading.Tasks;
1112
using Microsoft.CodeAnalysis;
1213

1314
namespace Roslyn.Utilities
@@ -18,10 +19,8 @@ namespace Roslyn.Utilities
1819
using Resources = WorkspacesResources;
1920
#endif
2021

21-
using SOW = ObjectWriter;
2222
using EncodingKind = ObjectWriter.EncodingKind;
2323
using Variant = ObjectWriter.Variant;
24-
using VariantKind = ObjectWriter.VariantKind;
2524

2625
/// <summary>
2726
/// An <see cref="ObjectReader"/> that deserializes objects from a byte stream.
@@ -45,14 +44,7 @@ internal sealed partial class ObjectReader : IDisposable
4544
private readonly ReaderReferenceMap<object> _objectReferenceMap;
4645
private readonly ReaderReferenceMap<string> _stringReferenceMap;
4746

48-
/// <summary>
49-
/// List of member values that object deserializers read from.
50-
/// </summary>
51-
private readonly ImmutableArray<Variant>.Builder _memberList;
52-
private int _indexInMemberList;
53-
54-
private static readonly ObjectPool<Stack<Construction>> s_constructionStackPool
55-
= new ObjectPool<Stack<Construction>>(() => new Stack<Construction>(20));
47+
private int _recursionDepth;
5648

5749
/// <summary>
5850
/// Creates a new instance of a <see cref="ObjectReader"/>.
@@ -71,8 +63,6 @@ private ObjectReader(
7163
_objectReferenceMap = new ReaderReferenceMap<object>();
7264
_stringReferenceMap = new ReaderReferenceMap<string>();
7365
_cancellationToken = cancellationToken;
74-
75-
_memberList = SOW.s_variantListPool.Allocate();
7666
}
7767

7868
/// <summary>
@@ -102,173 +92,58 @@ public void Dispose()
10292
{
10393
_objectReferenceMap.Dispose();
10494
_stringReferenceMap.Dispose();
105-
106-
ResetMemberList();
107-
SOW.s_variantListPool.Free(_memberList);
95+
_recursionDepth = 0;
10896
}
10997

110-
private void ResetMemberList()
111-
{
112-
_memberList.Clear();
113-
_indexInMemberList = 0;
114-
}
115-
116-
private Variant NextFromMemberList()
117-
=> _memberList[_indexInMemberList++];
118-
119-
private bool ShouldReadFromMemberList => _memberList.Count > 0;
120-
121-
public bool ReadBoolean()
122-
=> ShouldReadFromMemberList
123-
? NextFromMemberList().AsBoolean()
124-
: _reader.ReadBoolean();
125-
126-
public byte ReadByte()
127-
=> ShouldReadFromMemberList
128-
? NextFromMemberList().AsByte()
129-
: _reader.ReadByte();
130-
98+
public bool ReadBoolean() => _reader.ReadBoolean();
99+
public byte ReadByte() => _reader.ReadByte();
131100
// read as ushort because BinaryWriter fails on chars that are unicode surrogates
132-
public char ReadChar()
133-
=> ShouldReadFromMemberList
134-
? NextFromMemberList().AsChar()
135-
: (char)_reader.ReadUInt16();
136-
137-
public decimal ReadDecimal()
138-
=> ShouldReadFromMemberList
139-
? NextFromMemberList().AsDecimal()
140-
: _reader.ReadDecimal();
141-
142-
public double ReadDouble()
143-
=> ShouldReadFromMemberList
144-
? NextFromMemberList().AsDouble()
145-
: _reader.ReadDouble();
146-
147-
public float ReadSingle()
148-
=> ShouldReadFromMemberList
149-
? NextFromMemberList().AsSingle()
150-
: _reader.ReadSingle();
151-
152-
public int ReadInt32()
153-
=> ShouldReadFromMemberList
154-
? NextFromMemberList().AsInt32()
155-
: _reader.ReadInt32();
156-
157-
public long ReadInt64()
158-
=> ShouldReadFromMemberList
159-
? NextFromMemberList().AsInt64()
160-
: _reader.ReadInt64();
161-
162-
public sbyte ReadSByte()
163-
=> ShouldReadFromMemberList
164-
? NextFromMemberList().AsSByte()
165-
: _reader.ReadSByte();
166-
167-
public short ReadInt16()
168-
=> ShouldReadFromMemberList
169-
? NextFromMemberList().AsInt16()
170-
: _reader.ReadInt16();
171-
172-
public uint ReadUInt32()
173-
=> ShouldReadFromMemberList
174-
? NextFromMemberList().AsUInt32()
175-
: _reader.ReadUInt32();
176-
177-
public ulong ReadUInt64()
178-
=> ShouldReadFromMemberList
179-
? NextFromMemberList().AsUInt64()
180-
: _reader.ReadUInt64();
181-
182-
public ushort ReadUInt16()
183-
=> ShouldReadFromMemberList
184-
? NextFromMemberList().AsUInt16()
185-
: _reader.ReadUInt16();
186-
187-
public string ReadString()
188-
=> ShouldReadFromMemberList
189-
? ReadStringFromMemberList()
190-
: ReadStringValue();
101+
public char ReadChar() => (char)_reader.ReadUInt16();
102+
public decimal ReadDecimal() => _reader.ReadDecimal();
103+
public double ReadDouble() => _reader.ReadDouble();
104+
public float ReadSingle() => _reader.ReadSingle();
105+
public int ReadInt32() => _reader.ReadInt32();
106+
public long ReadInt64() => _reader.ReadInt64();
107+
public sbyte ReadSByte() => _reader.ReadSByte();
108+
public short ReadInt16() => _reader.ReadInt16();
109+
public uint ReadUInt32() => _reader.ReadUInt32();
110+
public ulong ReadUInt64() => _reader.ReadUInt64();
111+
public ushort ReadUInt16() => _reader.ReadUInt16();
112+
public string ReadString() => ReadStringValue();
191113

192114
public object ReadValue()
193115
{
194-
if (ShouldReadFromMemberList)
195-
{
196-
return ReadValueFromMemberList();
197-
}
198-
199-
var v = ReadVariant();
200-
return v.ToBoxedObject();
201-
}
202-
203-
private string ReadStringFromMemberList()
204-
{
205-
var next = NextFromMemberList();
206-
return next.Kind == VariantKind.Null
207-
? null
208-
: next.AsString();
209-
}
210-
211-
private object ReadValueFromMemberList()
212-
=> NextFromMemberList().ToBoxedObject();
213-
214-
/// <summary>
215-
/// Represents either a pending object or array construction.
216-
/// </summary>
217-
private struct Construction
218-
{
219-
/// <summary>
220-
/// The type of the object or the element type of the array.
221-
/// </summary>
222-
private readonly Type _type;
223-
224-
/// <summary>
225-
/// The number of values that must appear on the value stack (beyond the stack start) in order to
226-
/// trigger construction of the object or array instance.
227-
/// </summary>
228-
private readonly int _valueCount;
116+
var oldDepth = _recursionDepth;
117+
_recursionDepth++;
229118

230-
/// <summary>
231-
/// The size of the value stack before we started this construction.
232-
/// Only new values pushed onto the stack are used for this construction.
233-
/// </summary>
234-
private readonly int _stackStart;
235-
236-
/// <summary>
237-
/// The reader that constructs the object. Null if the construction is for an array.
238-
/// </summary>
239-
private readonly Func<ObjectReader, object> _reader;
240-
241-
/// <summary>
242-
/// The reference id of the object being constructed.
243-
/// </summary>
244-
private readonly int _id;
245-
246-
private Construction(Type type, int valueCount, int stackStart, Func<ObjectReader, object> reader, int id)
119+
object value;
120+
if (_recursionDepth % ObjectWriter.MaxRecursionDepth == 0)
247121
{
248-
_type = type;
249-
_valueCount = valueCount;
250-
_stackStart = stackStart;
251-
_reader = reader;
252-
_id = id;
122+
// If we're recursing too deep, move the work to another thread to do so we
123+
// don't blow the stack.
124+
var task = Task.Factory.StartNew(
125+
() => ReadValueWorker(),
126+
_cancellationToken,
127+
TaskCreationOptions.LongRunning,
128+
TaskScheduler.Default);
129+
task.Wait(_cancellationToken);
130+
value = task.Result;
253131
}
254-
255-
public bool CanConstruct(int stackCount)
132+
else
256133
{
257-
return stackCount == _stackStart + _valueCount;
134+
value = ReadValueWorker();
258135
}
259136

260-
public static Construction CreateObjectConstruction(Type type, int memberCount, int stackStart, Func<ObjectReader, object> reader, int id)
261-
{
262-
Debug.Assert(type != null);
263-
Debug.Assert(reader != null);
264-
return new Construction(type, memberCount, stackStart, reader, id);
265-
}
137+
_recursionDepth--;
138+
Debug.Assert(oldDepth == _recursionDepth);
266139

267-
public static Construction CreateArrayConstruction(Type elementType, int elementCount, int stackStart)
268-
{
269-
Debug.Assert(elementType != null);
270-
return new Construction(elementType, elementCount, stackStart, reader: null, id: 0);
271-
}
140+
return value;
141+
}
142+
143+
private object ReadValueWorker()
144+
{
145+
var v = ReadVariant();
146+
return v.ToBoxedObject();
272147
}
273148

274149
private Variant ReadVariant()

0 commit comments

Comments
 (0)