From c5f44ccee14595f53a2c15104401ff323ec6cd87 Mon Sep 17 00:00:00 2001 From: Karl Burtram Date: Wed, 12 Oct 2016 15:09:24 -0700 Subject: [PATCH] Switch from events to locks during parsing (#87) * Switch from event to locks in binding. * Remove unneeded null check. --- .../LanguageServices/AutoCompleteHelper.cs | 7 +- .../LanguageServices/BindingQueue.cs | 11 ++- .../ConnectedBindingContext.cs | 15 +++- .../LanguageServices/ConnectedBindingQueue.cs | 42 +++++----- .../LanguageServices/IBindingContext.cs | 4 +- .../LanguageServices/LanguageService.cs | 80 +++++++++---------- .../LanguageServices/QueueItem.cs | 1 - .../LanguageServices/ScriptParseInfo.cs | 7 +- .../LanguageServer/BindingQueueTests.cs | 5 +- 9 files changed, 87 insertions(+), 85 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs index 4eef4915..ad2846fb 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs @@ -4,6 +4,7 @@ // using System.Collections.Generic; +using System.Threading; using Microsoft.SqlServer.Management.SqlParser.Binder; using Microsoft.SqlServer.Management.SqlParser.Intellisense; using Microsoft.SqlServer.Management.SqlParser.Parser; @@ -572,12 +573,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices var scriptFile = AutoCompleteHelper.WorkspaceServiceInstance.Workspace.GetFile(info.OwnerUri); LanguageService.Instance.ParseAndBind(scriptFile, info); - if (scriptInfo.BuildingMetadataEvent.WaitOne(LanguageService.OnConnectionWaitTimeout)) + if (Monitor.TryEnter(scriptInfo.BuildingMetadataLock, LanguageService.OnConnectionWaitTimeout)) { try { - scriptInfo.BuildingMetadataEvent.Reset(); - QueueItem queueItem = bindingQueue.QueueBindingOperation( key: scriptInfo.ConnectionKey, bindingTimeout: AutoCompleteHelper.PrepopulateBindTimeout, @@ -631,7 +630,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } finally { - scriptInfo.BuildingMetadataEvent.Set(); + Monitor.Exit(scriptInfo.BuildingMetadataLock); } } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs index 2b165dc8..616bf17b 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs @@ -191,19 +191,22 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices continue; } + bool lockTaken = false; try { // prefer the queue item binding item, otherwise use the context default timeout int bindTimeout = queueItem.BindingTimeout ?? bindingContext.BindingTimeout; // handle the case a previous binding operation is still running - if (!bindingContext.BindingLocked.WaitOne(bindTimeout)) + if (!Monitor.TryEnter(bindingContext.BindingLock, bindTimeout)) { queueItem.Result = queueItem.TimeoutOperation(bindingContext); queueItem.ItemProcessed.Set(); continue; } + lockTaken = true; + // execute the binding operation object result = null; CancellationTokenSource cancelToken = new CancellationTokenSource(); @@ -237,7 +240,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } finally { - bindingContext.BindingLocked.Set(); + if (lockTaken) + { + Monitor.Exit(bindingContext.BindingLock); + } + queueItem.ItemProcessed.Set(); } diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingContext.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingContext.cs index 8851abe1..17cc6612 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingContext.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingContext.cs @@ -4,7 +4,6 @@ // using System; -using System.Threading; using Microsoft.SqlServer.Management.Common; using Microsoft.SqlServer.Management.SmoMetadataProvider; using Microsoft.SqlServer.Management.SqlParser.Binder; @@ -21,6 +20,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { private ParseOptions parseOptions; + private object bindingLock; + private ServerConnection serverConnection; /// @@ -28,7 +29,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// public ConnectedBindingContext() { - this.BindingLocked = new ManualResetEvent(initialState: true); + this.bindingLock = new object(); this.BindingTimeout = ConnectedBindingQueue.DefaultBindingTimeout; this.MetadataDisplayInfoProvider = new MetadataDisplayInfoProvider(); } @@ -72,9 +73,15 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public IBinder Binder { get; set; } /// - /// Gets or sets an event to signal if a binding operation is in progress + /// Gets the binding lock object /// - public ManualResetEvent BindingLocked { get; set; } + public object BindingLock + { + get + { + return this.bindingLock; + } + } /// /// Gets or sets the binding operation timeout in milliseconds diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingQueue.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingQueue.cs index c99f0cc6..ae6d8414 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingQueue.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ConnectedBindingQueue.cs @@ -63,22 +63,22 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices string connectionKey = GetConnectionContextKey(connInfo); IBindingContext bindingContext = this.GetOrCreateBindingContext(connectionKey); - try + lock (bindingContext.BindingLock) { - // increase the connection timeout to at least 30 seconds and and build connection string - // enable PersistSecurityInfo to handle issues in SMO where the connection context is lost in reconnections - int? originalTimeout = connInfo.ConnectionDetails.ConnectTimeout; - bool? originalPersistSecurityInfo = connInfo.ConnectionDetails.PersistSecurityInfo; - connInfo.ConnectionDetails.ConnectTimeout = Math.Max(DefaultMinimumConnectionTimeout, originalTimeout ?? 0); - connInfo.ConnectionDetails.PersistSecurityInfo = true; - string connectionString = ConnectionService.BuildConnectionString(connInfo.ConnectionDetails); - connInfo.ConnectionDetails.ConnectTimeout = originalTimeout; - connInfo.ConnectionDetails.PersistSecurityInfo = originalPersistSecurityInfo; - - // open a dedicated binding server connection - SqlConnection sqlConn = new SqlConnection(connectionString); - if (sqlConn != null) + try { + // increase the connection timeout to at least 30 seconds and and build connection string + // enable PersistSecurityInfo to handle issues in SMO where the connection context is lost in reconnections + int? originalTimeout = connInfo.ConnectionDetails.ConnectTimeout; + bool? originalPersistSecurityInfo = connInfo.ConnectionDetails.PersistSecurityInfo; + connInfo.ConnectionDetails.ConnectTimeout = Math.Max(DefaultMinimumConnectionTimeout, originalTimeout ?? 0); + connInfo.ConnectionDetails.PersistSecurityInfo = true; + string connectionString = ConnectionService.BuildConnectionString(connInfo.ConnectionDetails); + connInfo.ConnectionDetails.ConnectTimeout = originalTimeout; + connInfo.ConnectionDetails.PersistSecurityInfo = originalPersistSecurityInfo; + + // open a dedicated binding server connection + SqlConnection sqlConn = new SqlConnection(connectionString); sqlConn.Open(); // populate the binding context to work with the SMO metadata provider @@ -91,16 +91,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices bindingContext.Binder = BinderProvider.CreateBinder(bindingContext.SmoMetadataProvider); bindingContext.ServerConnection = serverConn; bindingContext.BindingTimeout = ConnectedBindingQueue.DefaultBindingTimeout; - bindingContext.IsConnected = true; + bindingContext.IsConnected = true; } - } - catch (Exception) - { - bindingContext.IsConnected = false; - } - finally - { - bindingContext.BindingLocked.Set(); + catch (Exception) + { + bindingContext.IsConnected = false; + } } return connectionKey; diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/IBindingContext.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/IBindingContext.cs index c83a28d7..0c4c7742 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/IBindingContext.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/IBindingContext.cs @@ -44,9 +44,9 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices IBinder Binder { get; set; } /// - /// Gets or sets an event to signal if a binding operation is in progress + /// Gets the binding lock object /// - ManualResetEvent BindingLocked { get; set; } + object BindingLock { get; } /// /// Gets or sets the binding operation timeout in milliseconds diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs index 889ad882..cc993e40 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs @@ -452,12 +452,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices // get or create the current parse info object ScriptParseInfo parseInfo = GetScriptParseInfo(scriptFile.ClientFilePath, createIfNotExists: true); - if (parseInfo.BuildingMetadataEvent.WaitOne(LanguageService.BindingTimeout)) + if (Monitor.TryEnter(parseInfo.BuildingMetadataLock, LanguageService.BindingTimeout)) { try { - parseInfo.BuildingMetadataEvent.Reset(); - if (connInfo == null || !parseInfo.IsConnected) { // parse current SQL file contents to retrieve a list of errors @@ -518,7 +516,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } finally { - parseInfo.BuildingMetadataEvent.Set(); + Monitor.Exit(parseInfo.BuildingMetadataLock); } } else @@ -538,11 +536,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices await Task.Run(() => { ScriptParseInfo scriptInfo = GetScriptParseInfo(info.OwnerUri, createIfNotExists: true); - if (scriptInfo.BuildingMetadataEvent.WaitOne(LanguageService.OnConnectionWaitTimeout)) + if (Monitor.TryEnter(scriptInfo.BuildingMetadataLock, LanguageService.OnConnectionWaitTimeout)) { try - { - scriptInfo.BuildingMetadataEvent.Reset(); + { scriptInfo.ConnectionKey = this.BindingQueue.AddConnectionContext(info); scriptInfo.IsConnected = true; @@ -556,7 +553,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { // Set Metadata Build event to Signal state. // (Tell Language Service that I am ready with Metadata Provider Object) - scriptInfo.BuildingMetadataEvent.Set(); + Monitor.Exit(scriptInfo.BuildingMetadataLock); } } @@ -631,9 +628,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); if (scriptParseInfo != null && scriptParseInfo.ParseResult != null) { - if (scriptParseInfo.BuildingMetadataEvent.WaitOne(LanguageService.FindCompletionStartTimeout)) + if (Monitor.TryEnter(scriptParseInfo.BuildingMetadataLock, LanguageService.FindCompletionStartTimeout)) { - scriptParseInfo.BuildingMetadataEvent.Reset(); try { QueueItem queueItem = this.BindingQueue.QueueBindingOperation( @@ -661,8 +657,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } finally { - scriptParseInfo.BuildingMetadataEvent.Set(); - } + Monitor.Exit(scriptParseInfo.BuildingMetadataLock); + } } } @@ -691,8 +687,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices this.currentCompletionParseInfo = null; - // Take a reference to the list at a point in time in case we update and replace the list - + // get the current script parse info object ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); if (connInfo == null || scriptParseInfo == null) { @@ -711,18 +706,18 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } if (scriptParseInfo.IsConnected - && scriptParseInfo.BuildingMetadataEvent.WaitOne(LanguageService.FindCompletionStartTimeout)) - { - scriptParseInfo.BuildingMetadataEvent.Reset(); - - QueueItem queueItem = this.BindingQueue.QueueBindingOperation( - key: scriptParseInfo.ConnectionKey, - bindingTimeout: LanguageService.BindingTimeout, - bindOperation: (bindingContext, cancelToken) => - { - CompletionItem[] completions = null; - try + && Monitor.TryEnter(scriptParseInfo.BuildingMetadataLock, LanguageService.FindCompletionStartTimeout)) + { + try + { + // queue the completion task with the binding queue + QueueItem queueItem = this.BindingQueue.QueueBindingOperation( + key: scriptParseInfo.ConnectionKey, + bindingTimeout: LanguageService.BindingTimeout, + bindOperation: (bindingContext, cancelToken) => { + CompletionItem[] completions = null; + // get the completion list from SQL Parser scriptParseInfo.CurrentSuggestions = Resolver.FindCompletions( scriptParseInfo.ParseResult, @@ -738,26 +733,27 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices scriptParseInfo.CurrentSuggestions, startLine, startColumn, - endColumn); - } - finally - { - scriptParseInfo.BuildingMetadataEvent.Set(); - } + endColumn); - return completions; - }, - timeoutOperation: (bindingContext) => + return completions; + }, + timeoutOperation: (bindingContext) => + { + return AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions); + }); + + queueItem.ItemProcessed.WaitOne(); + + var completionItems = queueItem.GetResultAsT(); + if (completionItems != null && completionItems.Length > 0) { - return AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions); - }); - - queueItem.ItemProcessed.WaitOne(); - var completionItems = queueItem.GetResultAsT(); - if (completionItems != null && completionItems.Length > 0) - { - return completionItems; + return completionItems; + } } + finally + { + Monitor.Exit(scriptParseInfo.BuildingMetadataLock); + } } return AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions); diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs index adf5fa18..931bf524 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs @@ -5,7 +5,6 @@ using System; using System.Threading; -using System.Threading.Tasks; namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ScriptParseInfo.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ScriptParseInfo.cs index 2c56d497..ec87ae6f 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ScriptParseInfo.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/ScriptParseInfo.cs @@ -4,7 +4,6 @@ // using System.Collections.Generic; -using System.Threading; using Microsoft.SqlServer.Management.SqlParser.Intellisense; using Microsoft.SqlServer.Management.SqlParser.Parser; @@ -15,14 +14,14 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// internal class ScriptParseInfo { - private ManualResetEvent buildingMetadataEvent = new ManualResetEvent(initialState: true); + private object buildingMetadataLock = new object(); /// /// Event which tells if MetadataProvider is built fully or not /// - public ManualResetEvent BuildingMetadataEvent + public object BuildingMetadataLock { - get { return this.buildingMetadataEvent; } + get { return this.buildingMetadataLock; } } /// diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs index 63181f67..d49aa9e5 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs @@ -4,7 +4,6 @@ // using System.Threading; -using System.Threading.Tasks; using Microsoft.SqlServer.Management.Common; using Microsoft.SqlServer.Management.SmoMetadataProvider; using Microsoft.SqlServer.Management.SqlParser.Binder; @@ -25,7 +24,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices { public TestBindingContext() { - this.BindingLocked = new ManualResetEvent(initialState: true); + this.BindingLock = new object(); this.BindingTimeout = 3000; } @@ -39,7 +38,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices public IBinder Binder { get; set; } - public ManualResetEvent BindingLocked { get; set; } + public object BindingLock { get; set; } public int BindingTimeout { get; set; }