Skip to content

Commit 1ec6e88

Browse files
authored
Merge pull request #67982 from sharwell/clear-request
Avoid retaining memory while waiting for changes
2 parents 2d50318 + 3bfbb7f commit 1ec6e88

File tree

2 files changed

+85
-6
lines changed

2 files changed

+85
-6
lines changed

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ await ComputeAndReportCurrentDiagnosticsAsync(
191191
}
192192
}
193193

194+
// Clear out the solution context to avoid retaining memory
195+
// https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1809058
196+
context.ClearSolutionContext();
197+
194198
// Some implementations of the spec will re-open requests as soon as we close them, spamming the server.
195199
// In those cases, we wait for the implementation to indicate that changes have occurred, then we close the connection
196200
// so that the client asks us again.

src/Features/LanguageServer/Protocol/Handler/RequestContext.cs

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
8+
using System.Runtime.CompilerServices;
89
using System.Threading;
910
using System.Threading.Tasks;
1011
using Microsoft.CodeAnalysis.Text;
@@ -44,22 +45,79 @@ internal readonly struct RequestContext
4445

4546
private readonly ILspServices _lspServices;
4647

48+
/// <summary>
49+
/// Provides backing storage for the LSP workspace used by this RequestContext instance, allowing it to be cleared
50+
/// on demand from all copies that may exist of this value type.
51+
/// </summary>
52+
/// <remarks>
53+
/// This field is only initialized for handlers that request solution context.
54+
/// </remarks>
55+
private readonly StrongBox<(Workspace Workspace, Solution Solution, Document? Document)>? _lspSolution;
56+
4757
/// <summary>
4858
/// The workspace this request is for, if applicable. This will be present if <see cref="Document"/> is
4959
/// present. It will be <see langword="null"/> if <c>requiresLSPSolution</c> is false.
5060
/// </summary>
51-
public readonly Workspace? Workspace;
61+
public Workspace? Workspace
62+
{
63+
get
64+
{
65+
if (_lspSolution is null)
66+
{
67+
// This request context never had a workspace instance
68+
return null;
69+
}
70+
71+
// The workspace is available unless it has been cleared by a call to ClearSolutionContext. Explicitly throw
72+
// for attempts to access this property after it has been manually cleared.
73+
return _lspSolution.Value.Workspace ?? throw new InvalidOperationException();
74+
}
75+
}
5276

5377
/// <summary>
5478
/// The solution state that the request should operate on, if the handler requires an LSP solution, or <see langword="null"/> otherwise
5579
/// </summary>
56-
public readonly Solution? Solution;
80+
public Solution? Solution
81+
{
82+
get
83+
{
84+
if (_lspSolution is null)
85+
{
86+
// This request context never had a solution instance
87+
return null;
88+
}
89+
90+
// The solution is available unless it has been cleared by a call to ClearSolutionContext. Explicitly throw
91+
// for attempts to access this property after it has been manually cleared.
92+
return _lspSolution.Value.Solution ?? throw new InvalidOperationException();
93+
}
94+
}
5795

5896
/// <summary>
5997
/// The document that the request is for, if applicable. This comes from the <see cref="TextDocumentIdentifier"/> returned from the handler itself via a call to
6098
/// <see cref="ITextDocumentIdentifierHandler{RequestType, TextDocumentIdentifierType}.GetTextDocumentIdentifier(RequestType)"/>.
6199
/// </summary>
62-
public readonly Document? Document;
100+
public Document? Document
101+
{
102+
get
103+
{
104+
if (_lspSolution is null)
105+
{
106+
// This request context never had a solution instance
107+
return null;
108+
}
109+
110+
// The solution is available unless it has been cleared by a call to ClearSolutionContext. Explicitly throw
111+
// for attempts to access this property after it has been manually cleared. Note that we can't rely on
112+
// Document being null for this check, because it is not always provided as part of the solution context.
113+
if (_lspSolution.Value.Workspace is null)
114+
{
115+
throw new InvalidOperationException();
116+
}
117+
118+
return _lspSolution.Value.Document;
119+
}
120+
}
63121

64122
/// <summary>
65123
/// The LSP server handling the request.
@@ -97,9 +155,18 @@ public RequestContext(
97155
ILspServices lspServices,
98156
CancellationToken queueCancellationToken)
99157
{
100-
Workspace = workspace;
101-
Document = document;
102-
Solution = solution;
158+
if (workspace is not null)
159+
{
160+
RoslynDebug.Assert(solution is not null);
161+
_lspSolution = new StrongBox<(Workspace Workspace, Solution Solution, Document? Document)>((workspace, solution, document));
162+
}
163+
else
164+
{
165+
RoslynDebug.Assert(solution is null);
166+
RoslynDebug.Assert(document is null);
167+
_lspSolution = null;
168+
}
169+
103170
_clientCapabilities = clientCapabilities;
104171
ServerKind = serverKind;
105172
SupportedLanguages = supportedLanguages;
@@ -228,6 +295,14 @@ public ValueTask StopTrackingAsync(Uri uri, CancellationToken cancellationToken)
228295
public bool IsTracking(Uri documentUri)
229296
=> _trackedDocuments.ContainsKey(documentUri);
230297

298+
public void ClearSolutionContext()
299+
{
300+
if (_lspSolution is null)
301+
return;
302+
303+
_lspSolution.Value = default;
304+
}
305+
231306
/// <summary>
232307
/// Logs an informational message.
233308
/// </summary>

0 commit comments

Comments
 (0)