diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index be8daacdf0597..e5c4e0b27f424 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -125,7 +125,7 @@ public async Task TestPreviewServices() using var previewWorkspace = new PreviewWorkspace(EditorTestCompositions.EditorFeatures.GetHostServices()); var persistentService = previewWorkspace.Services.SolutionServices.GetPersistentStorageService(); - await using var storage = await persistentService.GetStorageAsync(SolutionKey.ToSolutionKey(previewWorkspace.CurrentSolution), CancellationToken.None); + var storage = await persistentService.GetStorageAsync(SolutionKey.ToSolutionKey(previewWorkspace.CurrentSolution), CancellationToken.None); Assert.IsType(storage); } diff --git a/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs b/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs index 400df749f61f7..a501677df6270 100644 --- a/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs +++ b/src/Tools/AnalyzerRunner/IncrementalAnalyzerRunner.cs @@ -53,7 +53,7 @@ public async Task RunAsync(CancellationToken cancellationToken) if (usePersistentStorage) { var persistentStorageService = _workspace.Services.SolutionServices.GetPersistentStorageService(); - await using var persistentStorage = await persistentStorageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), cancellationToken).ConfigureAwait(false); + var persistentStorage = await persistentStorageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), cancellationToken).ConfigureAwait(false); if (persistentStorage is NoOpPersistentStorage) { throw new InvalidOperationException("Benchmark is not configured to use persistent storage."); diff --git a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs index aff0eef84138c..290022941f109 100644 --- a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs +++ b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs @@ -61,10 +61,9 @@ public void GlobalSetup() "); - var connectionPoolService = _workspace.ExportProvider.GetExportedValue(); var asyncListener = _workspace.ExportProvider.GetExportedValue().GetListener(FeatureAttribute.PersistentStorage); - _storageService = new SQLitePersistentStorageService(connectionPoolService, new StorageConfiguration(), asyncListener); + _storageService = new SQLitePersistentStorageService(new StorageConfiguration(), asyncListener); var solution = _workspace.CurrentSolution; _storage = _storageService.GetStorageAsync(SolutionKey.ToSolutionKey(solution), CancellationToken.None).AsTask().GetAwaiter().GetResult(); @@ -83,7 +82,6 @@ public void GlobalCleanup() } _document = null!; - _storage.Dispose(); _storage = null!; _storageService = null!; _workspace.Dispose(); diff --git a/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs b/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs index 7318e6a776139..bae7a906f62a3 100644 --- a/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs +++ b/src/Tools/IdeCoreBenchmarks/FindReferencesBenchmarks.cs @@ -84,10 +84,8 @@ private async Task LoadSolutionAsync() if (storageService == null) throw new ArgumentException("Couldn't get storage service"); - using (var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), CancellationToken.None)) - { - Console.WriteLine("Sucessfully got persistent storage instance"); - } + var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), CancellationToken.None); + Console.WriteLine("Successfully got persistent storage instance"); // There might be multiple projects with this name. That's ok. FAR goes and finds all the linked-projects // anyways to perform the search on all the equivalent symbols from them. So the end perf cost is the diff --git a/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs b/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs index f9c73e5a07fde..8b79dd838e9f9 100644 --- a/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs +++ b/src/Tools/IdeCoreBenchmarks/NavigateToBenchmarks.cs @@ -183,24 +183,24 @@ public async Task RunFullParallelIndexing() Console.WriteLine("Starting indexing"); var storageService = _workspace.Services.SolutionServices.GetPersistentStorageService(); - using (var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), CancellationToken.None)) - { - Console.WriteLine("Successfully got persistent storage instance"); - var start = DateTime.Now; - var indexTime = TimeSpan.Zero; - var tasks = _workspace.CurrentSolution.Projects.SelectMany(p => p.Documents).Select(d => Task.Run( - async () => - { - var tree = await d.GetSyntaxRootAsync(); - var stopwatch = SharedStopwatch.StartNew(); - await TopLevelSyntaxTreeIndex.GetIndexAsync(d, default); - await SyntaxTreeIndex.GetIndexAsync(d, default); - indexTime += stopwatch.Elapsed; - })).ToList(); - await Task.WhenAll(tasks); - Console.WriteLine("Indexing time : " + indexTime); - Console.WriteLine("Solution parallel: " + (DateTime.Now - start)); - } + var storage = await storageService.GetStorageAsync(SolutionKey.ToSolutionKey(_workspace.CurrentSolution), CancellationToken.None); + + Console.WriteLine("Successfully got persistent storage instance"); + var start = DateTime.Now; + var indexTime = TimeSpan.Zero; + var tasks = _workspace.CurrentSolution.Projects.SelectMany(p => p.Documents).Select(d => Task.Run( + async () => + { + var tree = await d.GetSyntaxRootAsync(); + var stopwatch = SharedStopwatch.StartNew(); + await TopLevelSyntaxTreeIndex.GetIndexAsync(d, default); + await SyntaxTreeIndex.GetIndexAsync(d, default); + indexTime += stopwatch.Elapsed; + })).ToList(); + await Task.WhenAll(tasks); + Console.WriteLine("Indexing time : " + indexTime); + Console.WriteLine("Solution parallel: " + (DateTime.Now - start)); + Console.WriteLine("DB flushed"); Console.ReadLine(); } diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs index 3aa4651e2bb30..4364077c46c92 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs @@ -23,7 +23,7 @@ namespace Microsoft.CodeAnalysis.UnitTests.WorkspaceServices { [UseExportProvider] - public abstract class AbstractPersistentStorageTests : IAsyncDisposable + public abstract class AbstractPersistentStorageTests : IDisposable { public enum Size { @@ -85,16 +85,10 @@ protected AbstractPersistentStorageTests() ThreadPool.SetMinThreads(Math.Max(workerThreads, NumThreads), completionPortThreads); } - internal abstract AbstractPersistentStorageService GetStorageService( - IMefHostExportProvider exportProvider, - IPersistentStorageConfiguration configuration, - IPersistentStorageFaultInjector? faultInjector, - string rootFolder); - - public async ValueTask DisposeAsync() + public void Dispose() { // This should cause the service to release the cached connection it maintains for the primary workspace - await (_storageService?.GetTestAccessor().ShutdownAsync() ?? Task.CompletedTask); + _storageService?.GetTestAccessor().Shutdown(); _persistentFolderRoot.Dispose(); } @@ -121,7 +115,7 @@ public async Task TestNullFilePaths() var streamName = "stream"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); var project = solution.Projects.First(); var document = project.Documents.First(); Assert.False(await storage.WriteStreamAsync(project, streamName, EncodeString(""))); @@ -142,14 +136,14 @@ public async Task CacheDirectoryInPathWithSingleQuote(Size size, bool withChecks var streamName1 = "PersistentService_Solution_WriteReadDifferentInstances1"; var streamName2 = "PersistentService_Solution_WriteReadDifferentInstances2"; - await using (var storage = await GetStorageAsync(solution, folder)) { + var storage = await GetStorageAsync(solution, folder); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); Assert.True(await storage.WriteStreamAsync(streamName2, EncodeString(GetData2(size)), GetChecksum2(withChecksum))); } - await using (var storage = await GetStorageAsync(solution, folder)) { + var storage = await GetStorageAsync(solution, folder); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum)))); Assert.Equal(GetData2(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName2, GetChecksum2(withChecksum)))); } @@ -163,14 +157,14 @@ public async Task PersistentService_Solution_WriteReadDifferentInstances(Size si var streamName1 = "PersistentService_Solution_WriteReadDifferentInstances1"; var streamName2 = "PersistentService_Solution_WriteReadDifferentInstances2"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); Assert.True(await storage.WriteStreamAsync(streamName2, EncodeString(GetData2(size)), GetChecksum2(withChecksum))); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum)))); Assert.Equal(GetData2(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName2, GetChecksum2(withChecksum)))); } @@ -184,16 +178,16 @@ public async Task PersistentService_Solution_WriteReadReopenSolution(Size size, var streamName1 = "PersistentService_Solution_WriteReadReopenSolution1"; var streamName2 = "PersistentService_Solution_WriteReadReopenSolution2"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); Assert.True(await storage.WriteStreamAsync(streamName2, EncodeString(GetData2(size)), GetChecksum2(withChecksum))); } solution = CreateOrOpenSolution(); - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum)))); Assert.Equal(GetData2(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName2, GetChecksum2(withChecksum)))); } @@ -207,7 +201,7 @@ public async Task PersistentService_Solution_WriteReadSameInstance(Size size, bo var streamName1 = "PersistentService_Solution_WriteReadSameInstance1"; var streamName2 = "PersistentService_Solution_WriteReadSameInstance2"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); Assert.True(await storage.WriteStreamAsync(streamName2, EncodeString(GetData2(size)), GetChecksum2(withChecksum))); @@ -223,7 +217,7 @@ public async Task PersistentService_Project_WriteReadSameInstance(Size size, boo var streamName1 = "PersistentService_Project_WriteReadSameInstance1"; var streamName2 = "PersistentService_Project_WriteReadSameInstance2"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); var project = solution.Projects.Single(); Assert.True(await storage.WriteStreamAsync(project, streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); @@ -241,7 +235,7 @@ public async Task PersistentService_Document_WriteReadSameInstance(Size size, bo var streamName1 = "PersistentService_Document_WriteReadSameInstance1"; var streamName2 = "PersistentService_Document_WriteReadSameInstance2"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); var document = solution.Projects.Single().Documents.Single(); Assert.True(await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); @@ -259,7 +253,7 @@ public async Task PersistentService_Solution_SimultaneousWrites([CombinatorialRa var streamName1 = "PersistentService_Solution_SimultaneousWrites1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); DoSimultaneousWrites(s => storage.WriteStreamAsync(streamName1, EncodeString(s))); var value = int.Parse(ReadStringToEnd(await storage.ReadStreamAsync(streamName1))); Assert.True(value >= 0); @@ -274,7 +268,7 @@ public async Task PersistentService_Project_SimultaneousWrites([CombinatorialRan var streamName1 = "PersistentService_Project_SimultaneousWrites1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); DoSimultaneousWrites(s => storage.WriteStreamAsync(solution.Projects.Single(), streamName1, EncodeString(s))); var value = int.Parse(ReadStringToEnd(await storage.ReadStreamAsync(solution.Projects.Single(), streamName1))); Assert.True(value >= 0); @@ -289,7 +283,7 @@ public async Task PersistentService_Document_SimultaneousWrites([CombinatorialRa var streamName1 = "PersistentService_Document_SimultaneousWrites1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); DoSimultaneousWrites(s => storage.WriteStreamAsync(solution.Projects.Single().Documents.Single(), streamName1, EncodeString(s))); var value = int.Parse(ReadStringToEnd(await storage.ReadStreamAsync(solution.Projects.Single().Documents.Single(), streamName1))); Assert.True(value >= 0); @@ -303,7 +297,7 @@ public async Task PersistentService_Solution_SimultaneousReads(Size size, bool w var solution = CreateOrOpenSolution(); var streamName1 = "PersistentService_Solution_SimultaneousReads1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); DoSimultaneousReads(async () => ReadStringToEnd(await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum))), GetData1(size)); } @@ -315,7 +309,7 @@ public async Task PersistentService_Project_SimultaneousReads(Size size, bool wi var solution = CreateOrOpenSolution(); var streamName1 = "PersistentService_Project_SimultaneousReads1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(solution.Projects.Single(), streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); DoSimultaneousReads(async () => ReadStringToEnd(await storage.ReadStreamAsync(solution.Projects.Single(), streamName1, GetChecksum1(withChecksum))), GetData1(size)); } @@ -328,7 +322,7 @@ public async Task PersistentService_Document_SimultaneousReads(Size size, bool w var solution = CreateOrOpenSolution(); var streamName1 = "PersistentService_Document_SimultaneousReads1"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(solution.Projects.Single().Documents.Single(), streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); DoSimultaneousReads(async () => ReadStringToEnd(await storage.ReadStreamAsync(solution.Projects.Single().Documents.Single(), streamName1, GetChecksum1(withChecksum))), GetData1(size)); } @@ -341,7 +335,7 @@ public async Task TestReadChecksumReturnsNullWhenNeverWritten([CombinatorialRang var streamName1 = "TestReadChecksumReturnsNullWhenNeverWritten"; - await using var storage = await GetStorageAsync(solution); + var storage = await GetStorageAsync(solution); Assert.False(await storage.ChecksumMatchesAsync(streamName1, s_checksum1)); } @@ -353,13 +347,13 @@ public async Task TestCanReadWithNullChecksumSomethingWrittenWithNonNullChecksum var streamName1 = "TestCanReadWithNullChecksumSomethingWrittenWithNonNullChecksum"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), s_checksum1)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName1, checksum: null))); } } @@ -372,13 +366,13 @@ public async Task TestCannotReadWithMismatchedChecksums(Size size, [Combinatoria var streamName1 = "TestCannotReadWithMismatchedChecksums"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), s_checksum1)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.Null(await storage.ReadStreamAsync(streamName1, s_checksum2)); } } @@ -391,13 +385,13 @@ public async Task TestCannotReadChecksumIfWriteDidNotIncludeChecksum(Size size, var streamName1 = "TestCannotReadChecksumIfWriteDidNotIncludeChecksum"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: null)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.False(await storage.ChecksumMatchesAsync(streamName1, s_checksum1)); } } @@ -410,13 +404,13 @@ public async Task TestReadChecksumProducesWrittenChecksum(Size size, [Combinator var streamName1 = "TestReadChecksumProducesWrittenChecksum"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: s_checksum1)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(streamName1, s_checksum1)); } } @@ -429,14 +423,14 @@ public async Task TestReadChecksumProducesLastWrittenChecksum1(Size size, [Combi var streamName1 = "TestReadChecksumProducesLastWrittenChecksum1"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: s_checksum1)); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: null)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.False(await storage.ChecksumMatchesAsync(streamName1, s_checksum1)); } } @@ -449,14 +443,14 @@ public async Task TestReadChecksumProducesLastWrittenChecksum2(Size size, [Combi var streamName1 = "TestReadChecksumProducesLastWrittenChecksum2"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: null)); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: s_checksum1)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(streamName1, s_checksum1)); } } @@ -469,14 +463,14 @@ public async Task TestReadChecksumProducesLastWrittenChecksum3(Size size, [Combi var streamName1 = "TestReadChecksumProducesLastWrittenChecksum3"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: s_checksum1)); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), checksum: s_checksum2)); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(streamName1, s_checksum2)); } } @@ -490,13 +484,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKey(Size size, [Combina var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); } @@ -511,13 +505,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocument(Size size, [Combinator var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); } @@ -532,13 +526,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKey(Size size, [Combinator var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); } @@ -553,13 +547,13 @@ public async Task TestOpenWithSolutionReadWithDocument(Size size, [Combinatorial var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); } @@ -574,13 +568,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKeyAndDocument1(Size size, var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); @@ -598,13 +592,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKeyAndDocument2(Size size, var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); @@ -622,13 +616,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKeyAndDocument1(Size si var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); @@ -646,13 +640,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKeyAndDocument2(Size si var streamName1 = "stream"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); @@ -670,13 +664,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKey_WriteWithSolutionKe var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); } @@ -691,13 +685,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocument_WriteWithSolutionKey(S var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); } @@ -712,13 +706,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKey_WriteWithSolutionKey(S var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); } @@ -733,13 +727,13 @@ public async Task TestOpenWithSolutionReadWithDocument_WriteWithSolutionKey(Size var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); } @@ -754,13 +748,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKeyAndDocument1_WriteWithS var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); @@ -778,13 +772,13 @@ public async Task TestOpenWithSolutionReadWithDocumentKeyAndDocument2_WriteWithS var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); @@ -802,13 +796,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKeyAndDocument1_WriteWi var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(DocumentKey.ToDocumentKey(document), streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(DocumentKey.ToDocumentKey(document), streamName1))); @@ -826,13 +820,13 @@ public async Task TestOpenWithSolutionKeyReadWithDocumentKeyAndDocument2_WriteWi var streamName1 = "stream"; - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); await storage.WriteStreamAsync(document, streamName1, EncodeString(GetData1(size)), checksum: s_checksum1); } - await using (var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution))) { + var storage = await GetStorageFromKeyAsync(solution.Workspace.Services, SolutionKey.ToSolutionKey(solution)); Assert.True(await storage.ChecksumMatchesAsync(document, streamName1, s_checksum1)); Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(document, streamName1))); @@ -859,13 +853,13 @@ public async Task PersistentService_ReadByteTwice(Size size, bool withChecksum, var solution = CreateOrOpenSolution(); var streamName1 = "PersistentService_ReadByteTwice"; - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum))); } - await using (var storage = await GetStorageAsync(solution)) { + var storage = await GetStorageAsync(solution); using var stream = await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum)); Contract.ThrowIfNull(stream); stream.ReadByte(); @@ -883,8 +877,8 @@ public async Task TestPersistSyntaxTreeIndex([CombinatorialRange(0, Iterations)] var document = solution.GetRequiredDocument(id); - await using (var storage = await GetStorageAsync(solution)) { + _ = await GetStorageAsync(solution); var index = await SyntaxTreeIndex.GetRequiredIndexAsync(document, default); await index.SaveAsync(document, _storageService!); @@ -903,8 +897,8 @@ public async Task TestPersistTopLevelSyntaxTreeIndex([CombinatorialRange(0, Iter var document = solution.GetRequiredDocument(id); - await using (var storage = await GetStorageAsync(solution)) { + _ = await GetStorageAsync(solution); var index = await TopLevelSyntaxTreeIndex.GetRequiredIndexAsync(document, default); await index.SaveAsync(document, _storageService!); @@ -981,6 +975,7 @@ private static void DoSimultaneousWrites(Func write) protected Solution CreateOrOpenSolution(TempDirectory? persistentFolder = null, bool nullPaths = false) { persistentFolder ??= _persistentFolder; + _storageService?.GetTestAccessor().Shutdown(); var solutionFile = persistentFolder.CreateOrOpenFile("Solution1.sln").WriteAllText(""); var info = SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), solutionFile.Path); @@ -1013,11 +1008,12 @@ internal async Task GetStorageAsync( { // If we handed out one for a previous test, we need to shut that down first persistentFolder ??= _persistentFolder; - await (_storageService?.GetTestAccessor().ShutdownAsync() ?? Task.CompletedTask); + _storageService?.GetTestAccessor().Shutdown(); var configuration = new MockPersistentStorageConfiguration(solution.Id, persistentFolder.Path, throwOnFailure); - _storageService = GetStorageService(solution.Workspace.Services.SolutionServices.ExportProvider, configuration, faultInjector, _persistentFolder.Path); - var storage = await _storageService.GetStorageAsync(SolutionKey.ToSolutionKey(solution), CancellationToken.None); + _storageService = (AbstractPersistentStorageService)solution.Workspace.Services.SolutionServices.GetPersistentStorageService(); + var storage = await _storageService.GetStorageAsync( + SolutionKey.ToSolutionKey(solution), configuration, faultInjector, CancellationToken.None); // If we're injecting faults, we expect things to be strange if (faultInjector == null) @@ -1031,12 +1027,11 @@ internal async Task GetStorageAsync( internal async Task GetStorageFromKeyAsync( HostWorkspaceServices services, SolutionKey solutionKey, IPersistentStorageFaultInjector? faultInjector = null) { - // If we handed out one for a previous test, we need to shut that down first - await (_storageService?.GetTestAccessor().ShutdownAsync() ?? Task.CompletedTask); var configuration = new MockPersistentStorageConfiguration(solutionKey.Id, _persistentFolder.Path, throwOnFailure: true); - _storageService = GetStorageService(services.SolutionServices.ExportProvider, configuration, faultInjector, _persistentFolder.Path); - var storage = await _storageService.GetStorageAsync(solutionKey, CancellationToken.None); + _storageService = (AbstractPersistentStorageService)services.SolutionServices.GetPersistentStorageService(); + var storage = await _storageService.GetStorageAsync( + solutionKey, configuration, faultInjector, CancellationToken.None); // If we're injecting faults, we expect things to be strange if (faultInjector == null) diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs index 11b59eeca6d11..6f1ef7522c274 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs @@ -22,13 +22,6 @@ namespace Microsoft.CodeAnalysis.UnitTests.WorkspaceServices /// public class SQLiteV2PersistentStorageTests : AbstractPersistentStorageTests { - internal override AbstractPersistentStorageService GetStorageService(IMefHostExportProvider exportProvider, IPersistentStorageConfiguration configuration, IPersistentStorageFaultInjector? faultInjector, string relativePathBase) - => new SQLitePersistentStorageService( - exportProvider.GetExports().Single().Value, - configuration, - exportProvider.GetExports().Single().Value.GetListener(FeatureAttribute.PersistentStorage), - faultInjector); - [Fact] public async Task TestCrashInNewConnection() { @@ -45,7 +38,7 @@ public async Task TestCrashInNewConnection() // Because instantiating the connection will fail, we will not get back // a working persistent storage. We are testing a fault recovery code path. - await using (var storage = await GetStorageAsync(solution, faultInjector: faultInjector, throwOnFailure: false)) + var storage = await GetStorageAsync(solution, faultInjector: faultInjector, throwOnFailure: false); using (var memStream = new MemoryStream()) using (var streamWriter = new StreamWriter(memStream)) { diff --git a/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs b/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs index e0a83cd9bd456..dd58ce4082086 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs @@ -64,7 +64,6 @@ internal partial class AbstractSyntaxIndex try { var storage = await storageService.GetStorageAsync(documentKey.Project.Solution, cancellationToken).ConfigureAwait(false); - await using var _ = storage.ConfigureAwait(false); // attempt to load from persisted state using var stream = await storage.ReadStreamAsync(documentKey, s_persistenceName, checksum, cancellationToken).ConfigureAwait(false); @@ -156,7 +155,6 @@ private async Task SaveAsync( try { var storage = await persistentStorageService.GetStorageAsync(solutionKey, cancellationToken).ConfigureAwait(false); - await using var _ = storage.ConfigureAwait(false); using (var stream = SerializableBytes.CreateWritableStream()) { diff --git a/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs b/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs index 22ccb6dcc399f..07564d99b7c1d 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs @@ -61,7 +61,6 @@ private static async Task LoadOrCreateAsync( var persistentStorageService = services.GetPersistentStorageService(); var storage = await persistentStorageService.GetStorageAsync(solutionKey, cancellationToken).ConfigureAwait(false); - await using var _ = storage.ConfigureAwait(false); using (var stream = SerializableBytes.CreateWritableStream()) { @@ -91,7 +90,6 @@ private static async Task LoadOrCreateAsync( var persistentStorageService = services.GetPersistentStorageService(); var storage = await persistentStorageService.GetStorageAsync(solutionKey, cancellationToken).ConfigureAwait(false); - await using var _ = storage.ConfigureAwait(false); // Get the unique key to identify our data. var key = PrefixSymbolTreeInfo + keySuffix; diff --git a/src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs index 57722a7b1a51c..15a9ef2cd4aa3 100644 --- a/src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs @@ -21,11 +21,8 @@ internal abstract partial class AbstractPersistentStorageService(IPersistentStor { protected readonly IPersistentStorageConfiguration Configuration = configuration; - /// - /// This lock guards all mutable fields in this type. - /// private readonly SemaphoreSlim _lock = new(initialCount: 1); - private ReferenceCountedDisposable? _currentPersistentStorage; + private IChecksummedPersistentStorage? _currentPersistentStorage; protected abstract string GetDatabaseFilePath(string workingFolderPath); @@ -34,73 +31,48 @@ internal abstract partial class AbstractPersistentStorageService(IPersistentStor /// to delete the database and retry opening one more time. If that fails again, the instance will be used. /// - protected abstract ValueTask TryOpenDatabaseAsync(SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, CancellationToken cancellationToken); - protected abstract bool ShouldDeleteDatabase(Exception exception); + protected abstract ValueTask TryOpenDatabaseAsync(SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken); - public async ValueTask GetStorageAsync(SolutionKey solutionKey, CancellationToken cancellationToken) + public ValueTask GetStorageAsync(SolutionKey solutionKey, CancellationToken cancellationToken) + => GetStorageAsync(solutionKey, this.Configuration, faultInjector: null, cancellationToken); + + public async ValueTask GetStorageAsync( + SolutionKey solutionKey, + IPersistentStorageConfiguration configuration, + IPersistentStorageFaultInjector? faultInjector, + CancellationToken cancellationToken) { if (solutionKey.FilePath == null) return NoOpPersistentStorage.GetOrThrow(solutionKey, Configuration.ThrowOnFailure); - // Without taking the lock, see if we can use the last storage system we were asked to create. Ensure we use a - // using so that if we don't take it we still release this reference count. - using var existing = _currentPersistentStorage?.TryAddReference(); - if (solutionKey == existing?.Target.SolutionKey) - { - // Success, we can use the current storage system. Ensure we increment the reference count again, so that - // this stays alive for the caller when the above reference count drops. - return PersistentStorageReferenceCountedDisposableWrapper.AddReferenceCountToAndCreateWrapper(existing); - } + // Without taking the lock, see if we can lookup a storage for this key. + var existing = _currentPersistentStorage; + if (existing?.SolutionKey == solutionKey) + return existing; - var workingFolder = Configuration.TryGetStorageLocation(solutionKey); + var workingFolder = configuration.TryGetStorageLocation(solutionKey); if (workingFolder == null) return NoOpPersistentStorage.GetOrThrow(solutionKey, Configuration.ThrowOnFailure); using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - // See if another thread set to the solution we care about while we were waiting on the lock. - using var current = _currentPersistentStorage?.TryAddReference(); - if (solutionKey != current?.Target.SolutionKey) - { - // We either don't have a current storage, or the current storage was pointing at some other solution. In - // either case, we want to create a new storage instance and point at that. If the current storage was - // point at some other solution, then lower its ref count (by calling DisposeAsync, outside of the lock) to - // indicate that *we* are letting go of it. - _ = DisposeStorageAsync(_currentPersistentStorage); + // Recheck if we have a storage for this key after taking the lock. + if (_currentPersistentStorage?.SolutionKey != solutionKey) + _currentPersistentStorage = await CreatePersistentStorageAsync(solutionKey, workingFolder, faultInjector, cancellationToken).ConfigureAwait(false); - // Create and cache a new storage instance associated with this particular solution. - // It will initially have a ref-count of 1 due to our reference to it. - _currentPersistentStorage = new ReferenceCountedDisposable( - await CreatePersistentStorageAsync(solutionKey, workingFolder, cancellationToken).ConfigureAwait(false)); - } - - // Now increment the reference count and return to our caller. The current ref count for this instance will - // be at least 2. Until all the callers *and* us decrement the refcounts, this instance will not be - // actually disposed. - Contract.ThrowIfNull(_currentPersistentStorage); - return PersistentStorageReferenceCountedDisposableWrapper.AddReferenceCountToAndCreateWrapper(_currentPersistentStorage); - } - - static async Task DisposeStorageAsync(ReferenceCountedDisposable? storage) - { - if (storage != null) - { - // Intentionally yield, so we don't execute any of this code outside the outer lock. - await Task.Yield().ConfigureAwait(false); - await storage.DisposeAsync().ConfigureAwait(false); - } + return _currentPersistentStorage; } } private async ValueTask CreatePersistentStorageAsync( - SolutionKey solutionKey, string workingFolderPath, CancellationToken cancellationToken) + SolutionKey solutionKey, string workingFolderPath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken) { // Attempt to create the database up to two times. The first time we may encounter // some sort of issue (like DB corruption). We'll then try to delete the DB and can // try to create it again. If we can't create it the second time, then there's nothing // we can do and we have to store things in memory. - var result = await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, cancellationToken).ConfigureAwait(false) ?? - await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, cancellationToken).ConfigureAwait(false); + var result = await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, faultInjector, cancellationToken).ConfigureAwait(false) ?? + await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, faultInjector, cancellationToken).ConfigureAwait(false); if (result != null) return result; @@ -111,12 +83,13 @@ private async ValueTask CreatePersistentStorageAs private async ValueTask TryCreatePersistentStorageAsync( SolutionKey solutionKey, string workingFolderPath, + IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken) { var databaseFilePath = GetDatabaseFilePath(workingFolderPath); try { - return await TryOpenDatabaseAsync(solutionKey, workingFolderPath, databaseFilePath, cancellationToken).ConfigureAwait(false); + return await TryOpenDatabaseAsync(solutionKey, workingFolderPath, databaseFilePath, faultInjector, cancellationToken).ConfigureAwait(false); } catch (Exception e) when (Recover(e)) { @@ -128,136 +101,14 @@ bool Recover(Exception ex) StorageDatabaseLogger.LogException(ex); if (Configuration.ThrowOnFailure) - { return false; - } - if (ShouldDeleteDatabase(ex)) - { - // this was not a normal exception that we expected during DB open. - // Report this so we can try to address whatever is causing this. - FatalError.ReportAndCatch(ex); - IOUtilities.PerformIO(() => Directory.Delete(Path.GetDirectoryName(databaseFilePath)!, recursive: true)); - } + // this was not a normal exception that we expected during DB open. + // Report this so we can try to address whatever is causing this. + FatalError.ReportAndCatch(ex); + IOUtilities.PerformIO(() => Directory.Delete(Path.GetDirectoryName(databaseFilePath)!, recursive: true)); return true; } } - - private async Task ShutdownAsync(CancellationToken cancellationToken) - { - ReferenceCountedDisposable? storage = null; - - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) - { - // We will transfer ownership in a thread-safe way out so we can dispose outside the lock - storage = _currentPersistentStorage; - _currentPersistentStorage = null; - } - - // Dispose storage outside of the lock. Note this only removes our reference count; clients who are still - // using this will still be holding a reference count. - if (storage != null) - await storage.DisposeAsync().ConfigureAwait(false); - } - - internal TestAccessor GetTestAccessor() - => new(this); - - internal readonly struct TestAccessor(AbstractPersistentStorageService service) - { - public Task ShutdownAsync() - => service.ShutdownAsync(CancellationToken.None); - } - - /// - /// A trivial wrapper that we can hand out for instances from the - /// that wraps the underlying singleton. - /// - private sealed class PersistentStorageReferenceCountedDisposableWrapper : IChecksummedPersistentStorage - { - private readonly ReferenceCountedDisposable _storage; - - private PersistentStorageReferenceCountedDisposableWrapper(ReferenceCountedDisposable storage) - => _storage = storage; - - public static IChecksummedPersistentStorage AddReferenceCountToAndCreateWrapper(ReferenceCountedDisposable storage) - { - // This should only be called from a caller that has a non-null storage that it - // already has a reference on. So .TryAddReference cannot fail. - return new PersistentStorageReferenceCountedDisposableWrapper(storage.TryAddReference() ?? throw ExceptionUtilities.Unreachable()); - } - - public void Dispose() - => _storage.Dispose(); - - public ValueTask DisposeAsync() - => _storage.DisposeAsync(); - - public SolutionKey SolutionKey - => _storage.Target.SolutionKey; - - public Task ChecksumMatchesAsync(string name, Checksum checksum, CancellationToken cancellationToken) - => _storage.Target.ChecksumMatchesAsync(name, checksum, cancellationToken); - - public Task ChecksumMatchesAsync(Project project, string name, Checksum checksum, CancellationToken cancellationToken) - => _storage.Target.ChecksumMatchesAsync(project, name, checksum, cancellationToken); - - public Task ChecksumMatchesAsync(Document document, string name, Checksum checksum, CancellationToken cancellationToken) - => _storage.Target.ChecksumMatchesAsync(document, name, checksum, cancellationToken); - - public Task ChecksumMatchesAsync(ProjectKey project, string name, Checksum checksum, CancellationToken cancellationToken) - => _storage.Target.ChecksumMatchesAsync(project, name, checksum, cancellationToken); - - public Task ChecksumMatchesAsync(DocumentKey document, string name, Checksum checksum, CancellationToken cancellationToken) - => _storage.Target.ChecksumMatchesAsync(document, name, checksum, cancellationToken); - - public Task ReadStreamAsync(string name, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(name, cancellationToken); - - public Task ReadStreamAsync(Project project, string name, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(project, name, cancellationToken); - - public Task ReadStreamAsync(Document document, string name, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(document, name, cancellationToken); - - public Task ReadStreamAsync(string name, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(name, checksum, cancellationToken); - - public Task ReadStreamAsync(Project project, string name, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(project, name, checksum, cancellationToken); - - public Task ReadStreamAsync(Document document, string name, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(document, name, checksum, cancellationToken); - - public Task ReadStreamAsync(ProjectKey project, string name, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(project, name, checksum, cancellationToken); - - public Task ReadStreamAsync(DocumentKey document, string name, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.ReadStreamAsync(document, name, checksum, cancellationToken); - - public Task WriteStreamAsync(string name, Stream stream, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(name, stream, cancellationToken); - - public Task WriteStreamAsync(Project project, string name, Stream stream, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(project, name, stream, cancellationToken); - - public Task WriteStreamAsync(Document document, string name, Stream stream, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(document, name, stream, cancellationToken); - - public Task WriteStreamAsync(string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(name, stream, checksum, cancellationToken); - - public Task WriteStreamAsync(Project project, string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(project, name, stream, checksum, cancellationToken); - - public Task WriteStreamAsync(Document document, string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(document, name, stream, checksum, cancellationToken); - - public Task WriteStreamAsync(ProjectKey projectKey, string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(projectKey, name, stream, checksum, cancellationToken); - - public Task WriteStreamAsync(DocumentKey documentKey, string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken) - => _storage.Target.WriteStreamAsync(documentKey, name, stream, checksum, cancellationToken); - } } diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/AbstractPersistentStorageService+SQLiteTestAccessor.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/AbstractPersistentStorageService+SQLiteTestAccessor.cs new file mode 100644 index 0000000000000..57f78ef598176 --- /dev/null +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/AbstractPersistentStorageService+SQLiteTestAccessor.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.CodeAnalysis.SQLite.v2; + +namespace Microsoft.CodeAnalysis.Storage; + +internal partial class AbstractPersistentStorageService +{ + internal TestAccessor GetTestAccessor() + => new(this); + + internal readonly struct TestAccessor(AbstractPersistentStorageService service) + { + public void Shutdown() + { + (service._currentPersistentStorage as SQLitePersistentStorage)?.DatabaseOwnership.Dispose(); + service._currentPersistentStorage = null; + } + } +} diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/Interop/SqlConnection.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/Interop/SqlConnection.cs index 440b6409cd496..c710a86dd43d2 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/Interop/SqlConnection.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/Interop/SqlConnection.cs @@ -21,16 +21,16 @@ namespace Microsoft.CodeAnalysis.SQLite.v2.Interop; /// Encapsulates a connection to a sqlite database. On construction an attempt will be made /// to open the DB if it exists, or create it if it does not. /// -/// Connections are considered relatively heavyweight and are pooled until the -/// is d. Connections can be used by different threads, -/// but only as long as they are used by one thread at a time. They are not safe for concurrent -/// use by several threads. +/// Connections are considered relatively heavyweight and are pooled (see ). Connections can be used by different +/// threads, but only as long as they are used by one thread at a time. They are not safe for concurrent use by several +/// threads. /// /// s can be created through the user of . /// These statements are cached for the lifetime of the connection and are only finalized /// (i.e. destroyed) when the connection is closed. /// -internal class SqlConnection +internal sealed class SqlConnection { // Cached UTF-8 (and null terminated) versions of the common strings we need to pass to sqlite. Used to prevent // having to convert these names to/from utf16 to UTF-8 on every call. Sqlite requires these be null terminated. diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool+PooledConnection.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool+PooledConnection.cs deleted file mode 100644 index 1b3750e7a9289..0000000000000 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool+PooledConnection.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using Microsoft.CodeAnalysis.SQLite.v2.Interop; - -namespace Microsoft.CodeAnalysis.SQLite.v2; - -internal partial class SQLiteConnectionPool -{ - internal readonly struct PooledConnection(SQLiteConnectionPool connectionPool, SqlConnection sqlConnection) : IDisposable - { - public readonly SqlConnection Connection = sqlConnection; - - public void Dispose() - => connectionPool.ReleaseConnection(Connection); - } -} diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPoolService.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPoolService.cs deleted file mode 100644 index bafe6a92c2e1f..0000000000000 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPoolService.cs +++ /dev/null @@ -1,159 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Composition; -using System.IO; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.ErrorReporting; -using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.Shared.Utilities; -using Microsoft.CodeAnalysis.SQLite.v2.Interop; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis.SQLite.v2; - -[Export] -[Shared] -internal sealed class SQLiteConnectionPoolService : IDisposable -{ - private const string LockFile = "db.lock"; - - private readonly object _gate = new(); - - /// - /// Maps from database file path to connection pool. - /// - /// - /// Access to this field is synchronized through . - /// - private readonly Dictionary> _connectionPools = []; - - [ImportingConstructor] - [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public SQLiteConnectionPoolService() - { - } - - /// - /// Use a to simulate a reader-writer lock. - /// Read operations are performed on the - /// and writes are performed on the . - /// - /// We use this as a condition of using the in-memory shared-cache sqlite DB. This DB - /// doesn't busy-wait when attempts are made to lock the tables in it, which can lead to - /// deadlocks. Specifically, consider two threads doing the following: - /// - /// Thread A starts a transaction that starts as a reader, and later attempts to perform a - /// write. Thread B is a writer (either started that way, or started as a reader and - /// promoted to a writer first). B holds a RESERVED lock, waiting for readers to clear so it - /// can start writing. A holds a SHARED lock (it's a reader) and tries to acquire RESERVED - /// lock (so it can start writing). The only way to make progress in this situation is for - /// one of the transactions to roll back. No amount of waiting will help, so when SQLite - /// detects this situation, it doesn't honor the busy timeout. - /// - /// To prevent this scenario, we control our access to the db explicitly with operations that - /// can concurrently read, and operations that exclusively write. - /// - /// All code that reads or writes from the db should go through this. - /// - public ConcurrentExclusiveSchedulerPair Scheduler { get; } = new(); - - public void Dispose() - { - lock (_gate) - { - foreach (var (_, pool) in _connectionPools) - pool.Dispose(); - - _connectionPools.Clear(); - } - } - - public ReferenceCountedDisposable? TryOpenDatabase( - string databaseFilePath, - IPersistentStorageFaultInjector? faultInjector, - Action initializer, - CancellationToken cancellationToken) - { - lock (_gate) - { - if (_connectionPools.TryGetValue(databaseFilePath, out var pool)) - { - return pool.TryAddReference() ?? throw ExceptionUtilities.Unreachable(); - } - - // try to get db ownership lock. if someone else already has the lock. it will throw - var ownershipLock = TryGetDatabaseOwnership(databaseFilePath); - if (ownershipLock == null) - { - return null; - } - - try - { - pool = new ReferenceCountedDisposable( - new SQLiteConnectionPool(this, faultInjector, databaseFilePath, ownershipLock)); - - pool.Target.Initialize(initializer, cancellationToken); - - // Place the initial ownership reference in _connectionPools, and return another - _connectionPools.Add(databaseFilePath, pool); - return pool.TryAddReference() ?? throw ExceptionUtilities.Unreachable(); - } - catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex, cancellationToken)) - { - if (pool is not null) - { - // Dispose of the connection pool, releasing the ownership lock. - pool.Dispose(); - } - else - { - // The storage was not created so nothing owns the lock. - // Dispose the lock to allow reuse. - ownershipLock.Dispose(); - } - - throw; - } - } - } - - /// - /// Returns null in the case where an IO exception prevented us from being able to acquire - /// the db lock file. - /// - private static IDisposable? TryGetDatabaseOwnership(string databaseFilePath) - { - return IOUtilities.PerformIO(() => - { - // make sure directory exist first. - EnsureDirectory(databaseFilePath); - - var directoryName = Path.GetDirectoryName(databaseFilePath); - Contract.ThrowIfNull(directoryName); - - return File.Open( - Path.Combine(directoryName, LockFile), - FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); - }, defaultValue: null); - } - - private static void EnsureDirectory(string databaseFilePath) - { - var directory = Path.GetDirectoryName(databaseFilePath); - Contract.ThrowIfNull(directory); - - if (Directory.Exists(directory)) - { - return; - } - - Directory.CreateDirectory(directory); - } -} diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage+PooledConnection.cs similarity index 55% rename from src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool.cs rename to src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage+PooledConnection.cs index f961393cb00e5..bc0cd558919b9 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLiteConnectionPool.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage+PooledConnection.cs @@ -3,61 +3,20 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; -using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.SQLite.v2.Interop; namespace Microsoft.CodeAnalysis.SQLite.v2; -internal sealed partial class SQLiteConnectionPool(SQLiteConnectionPoolService connectionPoolService, IPersistentStorageFaultInjector? faultInjector, string databasePath, IDisposable ownershipLock) : IDisposable +internal partial class SQLitePersistentStorage { - // We pool connections to the DB so that we don't have to take the hit of - // reconnecting. The connections also cache the prepared statements used - // to get/set data from the db. A connection is safe to use by one thread - // at a time, but is not safe for simultaneous use by multiple threads. - private readonly object _connectionGate = new(); - private readonly Stack _connectionsPool = new(); - - private readonly CancellationTokenSource _shutdownTokenSource = new(); - - internal void Initialize( - Action initializer, - CancellationToken cancellationToken) - { - // This is our startup path. No other code can be running. So it's safe for us to access a connection that - // can talk to the db without having to be on the reader/writer scheduler queue. - using var _ = GetPooledConnection(checkScheduler: false, out var connection); - - initializer(connection, cancellationToken); - } - - public void Dispose() + private readonly struct PooledConnection(SQLitePersistentStorage storage, SqlConnection sqlConnection) : IDisposable { - // Flush all pending writes so that all data our features wanted written - // are definitely persisted to the DB. - try - { - _shutdownTokenSource.Cancel(); - CloseWorker(); - } - finally - { - // let the lock go - ownershipLock.Dispose(); - } - } + public readonly SqlConnection Connection = sqlConnection; - private void CloseWorker() - { - lock (_connectionGate) - { - // Go through all our pooled connections and close them. - while (_connectionsPool.TryPop(out var connection)) - connection.Close_OnlyForUseBySQLiteConnectionPool(); - } + public void Dispose() + => storage.ReleaseConnection(Connection); } /// @@ -68,7 +27,7 @@ private void CloseWorker() /// longer in use. In particular, make sure to avoid letting a connection lease cross an /// boundary, as it will prevent code in the asynchronous operation from using the existing connection. /// - internal PooledConnection GetPooledConnection(out SqlConnection connection) + private PooledConnection GetPooledConnection(out SqlConnection connection) => GetPooledConnection(checkScheduler: true, out connection); /// @@ -81,8 +40,8 @@ private PooledConnection GetPooledConnection(bool checkScheduler, out SqlConnect if (checkScheduler) { var scheduler = TaskScheduler.Current; - if (scheduler != connectionPoolService.Scheduler.ConcurrentScheduler && scheduler != connectionPoolService.Scheduler.ExclusiveScheduler) - throw new InvalidOperationException($"Cannot get a connection to the DB unless running on one of {nameof(SQLiteConnectionPoolService)}'s schedulers"); + if (scheduler != this.Scheduler.ConcurrentScheduler && scheduler != this.Scheduler.ExclusiveScheduler) + throw new InvalidOperationException($"Cannot get a connection to the DB unless running on one of {nameof(SQLitePersistentStorage)}'s schedulers"); } var result = new PooledConnection(this, GetConnection()); @@ -100,7 +59,7 @@ private SqlConnection GetConnection() } // Otherwise create a new connection. - return SqlConnection.Create(faultInjector, databasePath); + return SqlConnection.Create(_faultInjector, this.DatabaseFile); } private void ReleaseConnection(SqlConnection connection) diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.Accessor.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.Accessor.cs index 4ea118171b220..0765d555aa450 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.Accessor.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.Accessor.cs @@ -163,13 +163,13 @@ private Optional ReadColumn( // We're reading. All current scenarios have this happening under the concurrent/read-only scheduler. // If this assert fires either a bug has been introduced, or there is a valid scenario for a writing // codepath to read a column and this assert should be adjusted. - Contract.ThrowIfFalse(TaskScheduler.Current == Storage._connectionPoolService.Scheduler.ConcurrentScheduler); + Contract.ThrowIfFalse(TaskScheduler.Current == this.Storage.Scheduler.ConcurrentScheduler); cancellationToken.ThrowIfCancellationRequested(); if (!Storage._shutdownTokenSource.IsCancellationRequested) { - using var _ = Storage._connectionPool.Target.GetPooledConnection(out var connection); + using var _ = this.Storage.GetPooledConnection(out var connection); // We're in the reading-only scheduler path, so we can't allow TryGetDatabaseId to write. Note that // this is ok, and actually provides the semantics we want. Specifically, we can be trying to read @@ -222,13 +222,13 @@ public Task WriteStreamAsync(TKey key, string name, Stream stream, Checksu private bool WriteStream(TKey key, string dataName, Stream stream, Checksum? checksum, CancellationToken cancellationToken) { // We're writing. This better always be under the exclusive scheduler. - Contract.ThrowIfFalse(TaskScheduler.Current == Storage._connectionPoolService.Scheduler.ExclusiveScheduler); + Contract.ThrowIfFalse(TaskScheduler.Current == this.Storage.Scheduler.ExclusiveScheduler); cancellationToken.ThrowIfCancellationRequested(); if (!Storage._shutdownTokenSource.IsCancellationRequested) { - using var _ = Storage._connectionPool.Target.GetPooledConnection(out var connection); + using var _ = this.Storage.GetPooledConnection(out var connection); // Determine the appropriate data-id to store this stream at. We already are running // with an exclusive write lock on the DB, so it's safe for us to write the data id to @@ -361,7 +361,7 @@ private void InsertOrReplaceBlobIntoWriteCache( ReadOnlySpan dataBytes) { // We're writing. This better always be under the exclusive scheduler. - Contract.ThrowIfFalse(TaskScheduler.Current == Storage._connectionPoolService.Scheduler.ExclusiveScheduler); + Contract.ThrowIfFalse(TaskScheduler.Current == this.Storage.Scheduler.ExclusiveScheduler); using (var resettableStatement = connection.GetResettableStatement( _insert_or_replace_into_writecache_table_values_0primarykey_1checksum_2data)) diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs index b7d7c0230257e..9ea33b38eff0f 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs @@ -4,11 +4,13 @@ using System; using System.Collections.Generic; +using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Shared.TestHooks; +using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.SQLite.Interop; using Microsoft.CodeAnalysis.SQLite.v2.Interop; using Microsoft.CodeAnalysis.Storage; @@ -23,14 +25,30 @@ namespace Microsoft.CodeAnalysis.SQLite.v2; /// internal sealed partial class SQLitePersistentStorage : AbstractPersistentStorage { + private const string LockFile = "db.lock"; + private readonly CancellationTokenSource _shutdownTokenSource = new(); private readonly string _solutionDirectory; - private readonly SQLiteConnectionPoolService _connectionPoolService; - private readonly ReferenceCountedDisposable _connectionPool; + // We pool connections to the DB so that we don't have to take the hit of + // reconnecting. The connections also cache the prepared statements used + // to get/set data from the db. A connection is safe to use by one thread + // at a time, but is not safe for simultaneous use by multiple threads. + private readonly object _connectionGate = new(); + private readonly Stack _connectionsPool = new(); private readonly Action _flushInMemoryDataToDisk; + /// + /// Lock file that ensures only one database is made per process per solution. + /// + public readonly IDisposable DatabaseOwnership; + + /// + /// For testing purposes. Allows us to test what happens when we fail to acquire the db lock file. + /// + private readonly IPersistentStorageFaultInjector? _faultInjector; + // Accessors that allow us to retrieve/store data into specific DB tables. The // core Accessor type has logic that we to share across all reading/writing, while // the derived types contain only enough logic to specify how to read/write from @@ -46,30 +64,48 @@ internal sealed partial class SQLitePersistentStorage : AbstractPersistentStorag private readonly string _select_star_from_string_table_where_0_limit_one = $"select * from {StringInfoTableName} where ({DataColumnName} = ?) limit 1"; private readonly string _select_star_from_string_table = $"select * from {StringInfoTableName}"; + /// + /// Use a to simulate a reader-writer lock. + /// Read operations are performed on the + /// and writes are performed on the . + /// + /// We use this as a condition of using the in-memory shared-cache sqlite DB. This DB + /// doesn't busy-wait when attempts are made to lock the tables in it, which can lead to + /// deadlocks. Specifically, consider two threads doing the following: + /// + /// Thread A starts a transaction that starts as a reader, and later attempts to perform a + /// write. Thread B is a writer (either started that way, or started as a reader and + /// promoted to a writer first). B holds a RESERVED lock, waiting for readers to clear so it + /// can start writing. A holds a SHARED lock (it's a reader) and tries to acquire RESERVED + /// lock (so it can start writing). The only way to make progress in this situation is for + /// one of the transactions to roll back. No amount of waiting will help, so when SQLite + /// detects this situation, it doesn't honor the busy timeout. + /// + /// To prevent this scenario, we control our access to the db explicitly with operations that + /// can concurrently read, and operations that exclusively write. + /// + /// All code that reads or writes from the db should go through this. + /// + private ConcurrentExclusiveSchedulerPair Scheduler { get; } = new(); + private SQLitePersistentStorage( - SQLiteConnectionPoolService connectionPoolService, SolutionKey solutionKey, string workingFolderPath, string databaseFile, IAsynchronousOperationListener asyncListener, - IPersistentStorageFaultInjector? faultInjector) + IPersistentStorageFaultInjector? faultInjector, + IDisposable databaseOwnership) : base(solutionKey, workingFolderPath, databaseFile) { Contract.ThrowIfNull(solutionKey.FilePath); _solutionDirectory = PathUtilities.GetDirectoryName(solutionKey.FilePath); - _connectionPoolService = connectionPoolService; _solutionAccessor = new SolutionAccessor(this); _projectAccessor = new ProjectAccessor(this); _documentAccessor = new DocumentAccessor(this); - // This assignment violates the declared non-nullability of _connectionPool, but the caller ensures that - // the constructed object is only used if the nullability post-conditions are met. - _connectionPool = connectionPoolService.TryOpenDatabase( - databaseFile, - faultInjector, - Initialize, - CancellationToken.None)!; + _faultInjector = faultInjector; + DatabaseOwnership = databaseOwnership; // Create a delay to batch up requests to flush. We'll won't flush more than every FlushAllDelayMS. _flushInMemoryDataToDisk = FlushInMemoryDataToDisk; @@ -81,49 +117,52 @@ private SQLitePersistentStorage( } public static SQLitePersistentStorage? TryCreate( - SQLiteConnectionPoolService connectionPoolService, SolutionKey solutionKey, string workingFolderPath, string databaseFile, IAsynchronousOperationListener asyncListener, IPersistentStorageFaultInjector? faultInjector) { - var sqlStorage = new SQLitePersistentStorage( - connectionPoolService, solutionKey, workingFolderPath, databaseFile, asyncListener, faultInjector); - if (sqlStorage._connectionPool is null) - { - // The connection pool failed to initialize + var databaseOwnership = TryGetDatabaseOwnership(databaseFile); + if (databaseOwnership is null) return null; - } - return sqlStorage; + var storage = new SQLitePersistentStorage( + solutionKey, workingFolderPath, databaseFile, asyncListener, faultInjector, databaseOwnership); + storage.Initialize(); + return storage; } - public override void Dispose() + /// + /// Returns null in the case where an IO exception prevented us from being able to acquire + /// the db lock file. + /// + private static IDisposable? TryGetDatabaseOwnership(string databaseFilePath) { - var task = DisposeAsync().AsTask(); - task.Wait(); - } + return IOUtilities.PerformIO(() => + { + // make sure directory exist first. + EnsureDirectory(databaseFilePath); - public override async ValueTask DisposeAsync() - { - try + var directoryName = Path.GetDirectoryName(databaseFilePath); + Contract.ThrowIfNull(directoryName); + + return File.Open( + Path.Combine(directoryName, LockFile), + FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + }, defaultValue: null); + + static void EnsureDirectory(string databaseFilePath) { - // Flush all pending writes so that all data our features wanted written are definitely - // persisted to the DB. - try - { - await FlushWritesOnCloseAsync().ConfigureAwait(false); - } - catch (Exception e) + var directory = Path.GetDirectoryName(databaseFilePath); + Contract.ThrowIfNull(directory); + + if (Directory.Exists(directory)) { - // Flushing may fail. We still have to close all our connections. - StorageDatabaseLogger.LogException(e); + return; } - } - finally - { - _connectionPool.Dispose(); + + Directory.CreateDirectory(directory); } } @@ -140,17 +179,11 @@ public static KeyValueLogMessage GetLogMessage(SqlException exception) d["Message"] = exception.Message; }); - private void Initialize(SqlConnection connection, CancellationToken cancellationToken) + private void Initialize() { - if (cancellationToken.IsCancellationRequested) - { - // Someone tried to get a connection *after* a call to Dispose the storage system - // happened. That should never happen. We only Dispose when the last ref to the - // storage system goes away. Once that happens, it's an error for there to be any - // future or existing consumers of the storage service. So nothing should be doing - // anything that wants to get an connection. - throw new InvalidOperationException(); - } + // This is our startup path. No other code can be running. So it's safe for us to access a connection that can + // talk to the db without having to be on the reader/writer scheduler queue. + using var _ = GetPooledConnection(checkScheduler: false, out var connection); // Ensure the database has tables for the types we care about. diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs index 9781d3eaf895f..9f1570f00f3b6 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs @@ -17,7 +17,6 @@ namespace Microsoft.CodeAnalysis.SQLite.v2; internal sealed class SQLitePersistentStorageService( - SQLiteConnectionPoolService connectionPoolService, IPersistentStorageConfiguration configuration, IAsynchronousOperationListener asyncListener) : AbstractPersistentStorageService(configuration), IWorkspaceService { @@ -25,13 +24,12 @@ internal sealed class SQLitePersistentStorageService( [method: ImportingConstructor] [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] internal sealed class ServiceFactory( - SQLiteConnectionPoolService connectionPoolService, IAsynchronousOperationListenerProvider asyncOperationListenerProvider) : IWorkspaceServiceFactory { private readonly IAsynchronousOperationListener _asyncListener = asyncOperationListenerProvider.GetListener(FeatureAttribute.PersistentStorage); public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) - => new SQLitePersistentStorageService(connectionPoolService, workspaceServices.GetRequiredService(), _asyncListener); + => new SQLitePersistentStorageService(workspaceServices.GetRequiredService(), _asyncListener); } private const string StorageExtension = "sqlite3"; @@ -61,18 +59,6 @@ private static bool TryInitializeLibrariesLazy() return true; } - private readonly IPersistentStorageFaultInjector? _faultInjector; - - public SQLitePersistentStorageService( - SQLiteConnectionPoolService connectionPoolService, - IPersistentStorageConfiguration configuration, - IAsynchronousOperationListener asyncListener, - IPersistentStorageFaultInjector? faultInjector) - : this(connectionPoolService, configuration, asyncListener) - { - _faultInjector = faultInjector; - } - protected override string GetDatabaseFilePath(string workingFolderPath) { Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(workingFolderPath)); @@ -80,7 +66,7 @@ protected override string GetDatabaseFilePath(string workingFolderPath) } protected override ValueTask TryOpenDatabaseAsync( - SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, CancellationToken cancellationToken) + SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken) { if (!TryInitializeLibraries()) { @@ -92,14 +78,10 @@ protected override string GetDatabaseFilePath(string workingFolderPath) return new(NoOpPersistentStorage.GetOrThrow(solutionKey, Configuration.ThrowOnFailure)); return new(SQLitePersistentStorage.TryCreate( - connectionPoolService, solutionKey, workingFolderPath, databaseFilePath, asyncListener, - _faultInjector)); + faultInjector)); } - - // Error occurred when trying to open this DB. Try to remove it so we can create a good DB. - protected override bool ShouldDeleteDatabase(Exception exception) => true; } diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs index 1988badfbe5db..e0d6ca9943104 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs @@ -29,47 +29,17 @@ private async ValueTask FlushInMemoryDataToDiskIfNotShutdownAsync(CancellationTo await PerformWriteAsync(_flushInMemoryDataToDisk, cancellationToken).ConfigureAwait(false); } - private Task FlushWritesOnCloseAsync() - { - // Issue a write task to write this all out to disk. - // - // Note: this only happens on close, so we don't try to avoid allocations here. - - return PerformWriteAsync( - () => - { - // Perform the actual write while having exclusive access to the scheduler. - FlushInMemoryDataToDisk(); - - // Now that we've done this, definitely cancel any further work. From this point on, it is now - // invalid for any codepaths to try to acquire a db connection for any purpose (beyond us - // disposing things below). - // - // This will also ensure that if we have a bg flush task still pending, when it wakes up it will - // see that we're shutdown and not proceed (and importantly won't acquire a connection). Because - // both the bg task and us run serialized, there is no way for it to miss this token - // cancellation. If it runs after us, then it sees this. If it runs before us, then we just - // block until it finishes. - // - // We don't have to worry about reads/writes getting connections either. - // The only way we can get disposed in the first place is if every user of this storage instance - // has released their ref on us. In that case, it would be an error on their part to ever try to - // read/write after releasing us. - _shutdownTokenSource.Cancel(); - }, CancellationToken.None); - } - private void FlushInMemoryDataToDisk() { // We're writing. This better always be under the exclusive scheduler. - Contract.ThrowIfFalse(TaskScheduler.Current == _connectionPoolService.Scheduler.ExclusiveScheduler); + Contract.ThrowIfFalse(TaskScheduler.Current == this.Scheduler.ExclusiveScheduler); // Don't flush from a bg task if we've been asked to shutdown. The shutdown logic in the storage service // will take care of the final writes to the main db. if (_shutdownTokenSource.IsCancellationRequested) return; - using var _ = _connectionPool.Target.GetPooledConnection(out var connection); + using var _ = this.GetPooledConnection(out var connection); var exception = connection.RunInTransaction(static state => { diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_StringIds.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_StringIds.cs index be276efa9a39a..c9c4e8c126b2c 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_StringIds.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_StringIds.cs @@ -47,8 +47,8 @@ internal partial class SQLitePersistentStorage { // We're reading or writing. This can be under either of our schedulers. Contract.ThrowIfFalse( - TaskScheduler.Current == _connectionPoolService.Scheduler.ExclusiveScheduler || - TaskScheduler.Current == _connectionPoolService.Scheduler.ConcurrentScheduler); + TaskScheduler.Current == this.Scheduler.ExclusiveScheduler || + TaskScheduler.Current == this.Scheduler.ConcurrentScheduler); // First, check if we can find that string in the string table. var stringId = TryGetStringIdFromDatabaseWorker(connection, value, canReturnNull: true); @@ -65,7 +65,7 @@ internal partial class SQLitePersistentStorage return null; // We're writing. This better always be under the exclusive scheduler. - Contract.ThrowIfFalse(TaskScheduler.Current == _connectionPoolService.Scheduler.ExclusiveScheduler); + Contract.ThrowIfFalse(TaskScheduler.Current == this.Scheduler.ExclusiveScheduler); // The string wasn't in the db string table. Add it. Note: this may fail if some // other thread/process beats us there as this table has a 'unique' constraint on the diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_Threading.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_Threading.cs index 72edf6a941550..0ff47ae62e10f 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_Threading.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_Threading.cs @@ -38,7 +38,7 @@ private Task PerformReadAsync(Func func, // Dispose method that restores ExecutionContext flow must run on the same thread where SuppressFlow was // originally run. using var _ = FlowControlHelper.TrySuppressFlow(); - return PerformTaskAsync(func, arg, _connectionPoolService.Scheduler.ConcurrentScheduler, cancellationToken); + return PerformTaskAsync(func, arg, this.Scheduler.ConcurrentScheduler, cancellationToken); } // Write tasks go to the exclusive-scheduler so they run exclusively of all other threading @@ -53,7 +53,7 @@ public Task PerformWriteAsync(Func func, // Dispose method that restores ExecutionContext flow must run on the same thread where SuppressFlow was // originally run. using var _ = FlowControlHelper.TrySuppressFlow(); - return PerformTaskAsync(func, arg, _connectionPoolService.Scheduler.ExclusiveScheduler, cancellationToken); + return PerformTaskAsync(func, arg, this.Scheduler.ExclusiveScheduler, cancellationToken); } public Task PerformWriteAsync(Action action, CancellationToken cancellationToken) diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/AbstractPersistentStorage.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/AbstractPersistentStorage.cs index eb2a66dbee458..81913cc81c0cb 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/AbstractPersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/AbstractPersistentStorage.cs @@ -43,9 +43,6 @@ private bool IsDisabled protected void DisableStorage() => Volatile.Write(ref _isDisabled, true); - public abstract void Dispose(); - public abstract ValueTask DisposeAsync(); - public abstract Task ChecksumMatchesAsync(string name, Checksum checksum, CancellationToken cancellationToken); public abstract Task ReadStreamAsync(string name, Checksum? checksum, CancellationToken cancellationToken); public abstract Task WriteStreamAsync(string name, Stream stream, Checksum? checksum, CancellationToken cancellationToken); diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorage.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorage.cs index 4b81585424cf5..9d5d44929fbc8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/IPersistentStorage.cs @@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.Host; /// disposal should always be preferred as the implementation of synchronous disposal may end up blocking the caller /// on async work. /// -public interface IPersistentStorage : IDisposable, IAsyncDisposable +public interface IPersistentStorage { Task ReadStreamAsync(string name, CancellationToken cancellationToken = default); Task ReadStreamAsync(Project project, string name, CancellationToken cancellationToken = default); diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/NoOpPersistentStorage.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/NoOpPersistentStorage.cs index d945acd65def7..c924931a44441 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/NoOpPersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/NoOpPersistentStorage.cs @@ -20,13 +20,6 @@ public static IChecksummedPersistentStorage GetOrThrow(SolutionKey solutionKey, ? throw new InvalidOperationException("Database was not supported") : new NoOpPersistentStorage(solutionKey); - public void Dispose() - { - } - - public ValueTask DisposeAsync() - => ValueTaskFactory.CompletedTask; - public Task ChecksumMatchesAsync(string name, Checksum checksum, CancellationToken cancellationToken) => SpecializedTasks.False; diff --git a/src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.Caching.cs b/src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.Caching.cs index 5db02e1df8e57..6bb8113b40e18 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.Caching.cs +++ b/src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.Caching.cs @@ -117,7 +117,6 @@ private static async Task CacheClassificationsAsync( var persistenceService = solution.Services.GetPersistentStorageService(); var storage = await persistenceService.GetStorageAsync(SolutionKey.ToSolutionKey(solution), cancellationToken).ConfigureAwait(false); - await using var _1 = storage.ConfigureAwait(false); if (storage == null) return; @@ -280,7 +279,6 @@ private async Task> TryReadCachedSemanticClassifi { var persistenceService = GetWorkspaceServices().GetPersistentStorageService(); var storage = await persistenceService.GetStorageAsync(documentKey.Project.Solution, cancellationToken).ConfigureAwait(false); - await using var _ = storage.ConfigureAwait(false); if (storage == null) return default;