diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs index c0244226..4eef4915 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs @@ -4,7 +4,6 @@ // using System.Collections.Generic; -using System.Threading.Tasks; using Microsoft.SqlServer.Management.SqlParser.Binder; using Microsoft.SqlServer.Management.SqlParser.Intellisense; using Microsoft.SqlServer.Management.SqlParser.Parser; @@ -22,6 +21,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// public static class AutoCompleteHelper { + private const int PrepopulateBindTimeout = 60000; + private static WorkspaceService workspaceServiceInstance; private static readonly string[] DefaultCompletionText = new string[] @@ -579,6 +580,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices QueueItem queueItem = bindingQueue.QueueBindingOperation( key: scriptInfo.ConnectionKey, + bindingTimeout: AutoCompleteHelper.PrepopulateBindTimeout, bindOperation: (bindingContext, cancelToken) => { // parse a simple statement that returns common metadata @@ -619,7 +621,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices // this forces lazy evaluation of the suggestion metadata AutoCompleteHelper.ConvertDeclarationsToCompletionItems(suggestions, 1, 6, 6); - return Task.FromResult(null as object); + return null; }); queueItem.ItemProcessed.WaitOne(); diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs index 89b5e0c7..2b165dc8 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs @@ -59,8 +59,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// public QueueItem QueueBindingOperation( string key, - Func> bindOperation, - Func> timeoutOperation = null, + Func bindOperation, + Func timeoutOperation = null, int? bindingTimeout = null) { // don't add null operations to the binding queue @@ -184,53 +184,48 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices continue; } - try + IBindingContext bindingContext = GetOrCreateBindingContext(queueItem.Key); + if (bindingContext == null) { - IBindingContext bindingContext = GetOrCreateBindingContext(queueItem.Key); - if (bindingContext == null) - { - queueItem.ItemProcessed.Set(); - continue; - } - + queueItem.ItemProcessed.Set(); + continue; + } + + 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 + + // handle the case a previous binding operation is still running if (!bindingContext.BindingLocked.WaitOne(bindTimeout)) { - queueItem.ResultsTask = Task.Run(() => - { - var timeoutTask = queueItem.TimeoutOperation(bindingContext); - timeoutTask.ContinueWith((obj) => queueItem.ItemProcessed.Set()); - return timeoutTask.Result; - }); - + queueItem.Result = queueItem.TimeoutOperation(bindingContext); + queueItem.ItemProcessed.Set(); continue; } // execute the binding operation - CancellationTokenSource cancelToken = new CancellationTokenSource(); - queueItem.ResultsTask = queueItem.BindOperation( - bindingContext, - cancelToken.Token); - - // set notification events once the binding operation task completes - queueItem.ResultsTask.ContinueWith((obj) => - { - queueItem.ItemProcessed.Set(); - bindingContext.BindingLocked.Set(); - }); + object result = null; + CancellationTokenSource cancelToken = new CancellationTokenSource(); + var bindTask = Task.Run(() => + { + result = queueItem.BindOperation( + bindingContext, + cancelToken.Token); + }); // check if the binding tasks completed within the binding timeout - if (!queueItem.ResultsTask.Wait(bindTimeout)) + if (bindTask.Wait(bindTimeout)) { + queueItem.Result = result; + } + else + { // if the task didn't complete then call the timeout callback if (queueItem.TimeoutOperation != null) { cancelToken.Cancel(); - queueItem.ResultsTask = queueItem.TimeoutOperation(bindingContext); - queueItem.ResultsTask.ContinueWith((obj) => queueItem.ItemProcessed.Set()); + queueItem.Result = queueItem.TimeoutOperation(bindingContext); } } } @@ -238,7 +233,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { // catch and log any exceptions raised in the binding calls // set item processed to avoid deadlocks - Logger.Write(LogLevel.Error, "Binding queue threw exception " + ex.ToString()); + Logger.Write(LogLevel.Error, "Binding queue threw exception " + ex.ToString()); + } + finally + { + bindingContext.BindingLocked.Set(); queueItem.ItemProcessed.Set(); } diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs index fe196f7d..889ad882 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs @@ -39,7 +39,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices internal const int HoverTimeout = 3000; - internal const int FindCompletionsTimeout = 3000; + internal const int BindingTimeout = 3000; internal const int FindCompletionStartTimeout = 50; @@ -452,7 +452,7 @@ 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.FindCompletionsTimeout)) + if (parseInfo.BuildingMetadataEvent.WaitOne(LanguageService.BindingTimeout)) { try { @@ -472,6 +472,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { QueueItem queueItem = this.BindingQueue.QueueBindingOperation( key: parseInfo.ConnectionKey, + bindingTimeout: LanguageService.BindingTimeout, bindOperation: (bindingContext, cancelToken) => { try @@ -503,7 +504,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices Logger.Write(LogLevel.Error, "Unknown exception during parsing " + ex.ToString()); } - return Task.FromResult(null as object); + return null; }); queueItem.ItemProcessed.WaitOne(); @@ -520,6 +521,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices parseInfo.BuildingMetadataEvent.Set(); } } + else + { + Logger.Write(LogLevel.Warning, "Binding metadata lock timeout in ParseAndBind"); + } return parseInfo.ParseResult; } @@ -644,13 +649,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices bindingContext.MetadataDisplayInfoProvider); // convert from the parser format to the VS Code wire format - return Task.FromResult( - AutoCompleteHelper.ConvertQuickInfoToHover( + return AutoCompleteHelper.ConvertQuickInfoToHover( quickInfo, startLine, startColumn, - endColumn - ) as object); + endColumn); }); queueItem.ItemProcessed.WaitOne(); @@ -714,6 +717,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices QueueItem queueItem = this.BindingQueue.QueueBindingOperation( key: scriptParseInfo.ConnectionKey, + bindingTimeout: LanguageService.BindingTimeout, bindOperation: (bindingContext, cancelToken) => { CompletionItem[] completions = null; @@ -741,12 +745,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices scriptParseInfo.BuildingMetadataEvent.Set(); } - return Task.FromResult(completions as object); + return completions; }, timeoutOperation: (bindingContext) => { - return Task.FromResult( - AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions) as object); + return AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions); }); queueItem.ItemProcessed.WaitOne(); diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs index 2ec25e1a..adf5fa18 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/QueueItem.cs @@ -30,12 +30,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// /// Gets or sets the bind operation callback method /// - public Func> BindOperation { get; set; } + public Func BindOperation { get; set; } /// /// Gets or sets the timeout operation to call if the bind operation doesn't finish within timeout period /// - public Func> TimeoutOperation { get; set; } + public Func TimeoutOperation { get; set; } /// /// Gets or sets an event to signal when this queue item has been processed @@ -43,10 +43,9 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public ManualResetEvent ItemProcessed { get; set; } /// - /// Gets or sets the task that was used to execute this queue item. - /// This allows the queuer to retrieve the execution result. + /// Gets or sets the result of the queued task /// - public Task ResultsTask { get; set; } + public object Result { get; set; } /// /// Gets or sets the binding operation timeout in milliseconds @@ -54,13 +53,13 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public int? BindingTimeout { get; set; } /// - /// Converts the result of the execution task to type T + /// Converts the result of the execution to type T /// public T GetResultAsT() where T : class { - var task = this.ResultsTask; - return (task != null && task.IsCompleted && task.Result != null) - ? task.Result as T + //var task = this.ResultsTask; + return (this.Result != null) + ? this.Result as T : null; } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs index 5d79e292..63181f67 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/LanguageServer/BindingQueueTests.cs @@ -89,45 +89,29 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices /// /// Test bind operation callback /// - private Task TestBindOperation( + private object TestBindOperation( IBindingContext bindContext, CancellationToken cancelToken) - { - return Task.Run(() => + { + cancelToken.WaitHandle.WaitOne(this.bindCallbackDelay); + this.isCancelationRequested = cancelToken.IsCancellationRequested; + if (!this.isCancelationRequested) { - cancelToken.WaitHandle.WaitOne(this.bindCallbackDelay); - this.isCancelationRequested = cancelToken.IsCancellationRequested; - if (!this.isCancelationRequested) - { - ++this.bindCallCount; - } - return new CompletionItem[0] as object; - }); + ++this.bindCallCount; + } + return new CompletionItem[0]; } /// /// Test callback for the bind timeout operation /// - private Task TestTimeoutOperation( + private object TestTimeoutOperation( IBindingContext bindingContext) { ++this.timeoutCallCount; - return Task.FromResult(new CompletionItem[0] as object); + return new CompletionItem[0]; } - /// - /// Runs for a few seconds to allow the queue to pump any requests - /// - private void WaitForQueue(int delay = 5000) - { - int step = 50; - int steps = delay / step + 1; - for (int i = 0; i < steps; ++i) - { - Thread.Sleep(step); - } - } - /// /// Queues a single task /// @@ -141,7 +125,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices bindOperation: TestBindOperation, timeoutOperation: TestTimeoutOperation); - WaitForQueue(); + Thread.Sleep(1000); this.bindingQueue.StopQueueProcessor(15000); @@ -166,7 +150,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices timeoutOperation: TestTimeoutOperation); } - WaitForQueue(); + Thread.Sleep(2000); this.bindingQueue.StopQueueProcessor(15000); @@ -183,14 +167,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices { InitializeTestSettings(); - this.bindCallbackDelay = 10000; + this.bindCallbackDelay = 1000; this.bindingQueue.QueueBindingOperation( key: "testkey", + bindingTimeout: bindCallbackDelay / 2, bindOperation: TestBindOperation, timeoutOperation: TestTimeoutOperation); - WaitForQueue(this.bindCallbackDelay + 2000); + Thread.Sleep(this.bindCallbackDelay + 100); this.bindingQueue.StopQueueProcessor(15000);