From db32cee0d772fa2e23f2f95f77ae43caae53c821 Mon Sep 17 00:00:00 2001 From: Kate Shin Date: Sat, 21 Oct 2017 11:05:31 -0700 Subject: [PATCH] Fix filebrowser to handle cancellation and multiple open/expand requests (#499) * add cancel filebrowser request * update expand request contract * cleanup * add queue * add expand queue and update cancellation * dispose cancelsource * remove unnecessary field * cleanup using directives * address pr comment * more changes * change contract --- .../Contracts/FileBrowserOpenRequest.cs | 5 + .../FileBrowser/FileBrowserOperation.cs | 136 +++++++++++++----- .../FileBrowser/FileBrowserService.cs | 136 +++++++++++++----- .../LanguageServices/BindingQueue.cs | 37 ++++- 4 files changed, 238 insertions(+), 76 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/Contracts/FileBrowserOpenRequest.cs b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/Contracts/FileBrowserOpenRequest.cs index 52cb2af5..66597c53 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/Contracts/FileBrowserOpenRequest.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/Contracts/FileBrowserOpenRequest.cs @@ -26,6 +26,11 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts /// File extension filter (e.g. *.bak) /// public string[] FileFilters; + + /// + /// True if this is a request to change file filter + /// + public bool ChangeFilter; } /// diff --git a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserOperation.cs b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserOperation.cs index 276bd2e9..f5b99004 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserOperation.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserOperation.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Data.SqlClient; using System.Globalization; using System.Text.RegularExpressions; +using System.Threading; using Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts; namespace Microsoft.SqlTools.ServiceLayer.FileBrowser @@ -15,11 +16,14 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser /// /// Implementation for file browser operation /// - internal class FileBrowserOperation : FileBrowserBase + internal class FileBrowserOperation : FileBrowserBase, IDisposable { private FileTree fileTree; private string expandPath; private string[] fileFilters; + private bool fileTreeCreated; + private CancellationTokenSource cancelSource; + private CancellationToken cancelToken; #region Constructors @@ -29,6 +33,8 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser public FileBrowserOperation() { this.fileTree = new FileTree(); + this.cancelSource = new CancellationTokenSource(); + this.cancelToken = cancelSource.Token; } /// @@ -39,15 +45,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser public FileBrowserOperation(SqlConnection connectionInfo, string expandPath, string[] fileFilters = null): this() { this.sqlConnection = connectionInfo; - this.expandPath = expandPath; - if (fileFilters == null) - { - this.fileFilters = new string[1] { "*" }; - } - else - { - this.fileFilters = fileFilters; - } + this.Initialize(expandPath, fileFilters); } #endregion @@ -70,13 +68,65 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser } } + public bool FileTreeCreated + { + get + { + return this.fileTreeCreated; + } + } + + public SqlConnection SqlConnection + { + get + { + return this.sqlConnection; + } + } + + public bool IsCancellationRequested + { + get + { + return this.cancelToken.IsCancellationRequested; + } + } + + public void Cancel() + { + this.cancelSource.Cancel(); + } #endregion + public void Initialize(string expandPath, string[] fileFilters) + { + this.expandPath = expandPath; + if (fileFilters == null) + { + this.fileFilters = new string[1] { "*" }; + } + else + { + this.fileFilters = fileFilters; + } + } + + public void Dispose() + { + if (this.sqlConnection != null) + { + this.sqlConnection.Close(); + } + this.cancelSource.Dispose(); + } + public void PopulateFileTree() { + this.fileTreeCreated = false; this.PathSeparator = GetPathSeparator(this.Enumerator, this.sqlConnection); PopulateDrives(); ExpandSelectedNode(this.expandPath); + this.fileTreeCreated = true; } /// @@ -94,6 +144,11 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser foreach (string dir in dirs) { + if (cancelToken.IsCancellationRequested) + { + break; + } + FileTreeNode currentNode = null; foreach (FileTreeNode node in currentChildren) { @@ -131,10 +186,42 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser } } - public List GetChildren(string filepath) + public void PopulateDrives() { + bool first = true; + if (!cancelToken.IsCancellationRequested) + { + foreach (var fileInfo in EnumerateDrives(Enumerator, sqlConnection)) + { + if (cancelToken.IsCancellationRequested) + { + break; + } + + // Windows drive letter paths have a '\' at the end. Linux drive paths won't have a '\'. + var node = new FileTreeNode + { + Name = Convert.ToString(fileInfo.path, CultureInfo.InvariantCulture).TrimEnd('\\'), + FullPath = fileInfo.path + }; + + this.fileTree.RootNode.AddChildNode(node); + + if (first) + { + this.fileTree.SelectedNode = node; + first = false; + } + + node.Children = this.GetChildren(node.FullPath); + } + } + } + + public List GetChildren(string filePath) + { List children = new List(); - foreach (var file in EnumerateFilesInFolder(Enumerator, sqlConnection, filepath)) + foreach (var file in EnumerateFilesInFolder(Enumerator, sqlConnection, filePath)) { bool isFile = !string.IsNullOrEmpty(file.fileName); FileTreeNode treeNode = new FileTreeNode(); @@ -157,34 +244,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser children.Add(treeNode); } } - return children; } - public void PopulateDrives() - { - bool first = true; - foreach (var fileInfo in EnumerateDrives(Enumerator, sqlConnection)) - { - // Windows drive letter paths have a '\' at the end. Linux drive paths won't have a '\'. - var node = new FileTreeNode - { - Name = Convert.ToString(fileInfo.path, CultureInfo.InvariantCulture).TrimEnd('\\'), - FullPath = fileInfo.path - }; - - this.fileTree.RootNode.AddChildNode(node); - - if (first) - { - this.fileTree.SelectedNode = node; - first = false; - } - - node.Children = this.GetChildren(node.FullPath); - } - } - /// /// Filter a filename based on the full mask provide. The full mask may be a collection a masks seperated by semi-colons. /// For example: *; *.txt diff --git a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserService.cs b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserService.cs index de555582..cd3c2da0 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/FileBrowser/FileBrowserService.cs @@ -5,14 +5,13 @@ using System; using System.Collections.Concurrent; -using System.Data.Common; using System.Data.SqlClient; using System.Threading.Tasks; using Microsoft.SqlTools.Hosting.Protocol; using Microsoft.SqlTools.ServiceLayer.Connection; -using Microsoft.SqlTools.ServiceLayer.Connection.ReliableConnection; using Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts; using Microsoft.SqlTools.ServiceLayer.Hosting; +using Microsoft.SqlTools.ServiceLayer.LanguageServices; using Microsoft.SqlTools.ServiceLayer.Utility; namespace Microsoft.SqlTools.ServiceLayer.FileBrowser @@ -20,7 +19,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser /// /// Main class for file browser service /// - public sealed class FileBrowserService + public sealed class FileBrowserService: IDisposable { private static readonly Lazy LazyInstance = new Lazy(() => new FileBrowserService()); public static FileBrowserService Instance => LazyInstance.Value; @@ -29,6 +28,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser private readonly ConcurrentDictionary ownerToFileBrowserMap = new ConcurrentDictionary(); private readonly ConcurrentDictionary validatePathsCallbackMap = new ConcurrentDictionary(); private ConnectionService connectionService; + private ConnectedBindingQueue expandNodeQueue = new ConnectedBindingQueue(needsMetadata: false); + private static int DefaultExpandTimeout = 120000; + private string serviceName = "FileBrowser"; /// /// Signature for callback method that validates the selected file paths @@ -43,6 +45,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser if (connectionService == null) { connectionService = ConnectionService.Instance; + connectionService.RegisterConnectedQueue(this.serviceName, this.expandNodeQueue); } return connectionService; } @@ -133,54 +136,98 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser FileBrowserOperation removedOperation; response.Succeeded = ownerToFileBrowserMap.TryRemove(fileBrowserParams.OwnerUri, out removedOperation); + if (removedOperation != null && this.expandNodeQueue != null) + { + bool hasPendingQueueItems = this.expandNodeQueue.HasPendingQueueItems; + if (removedOperation.FileTreeCreated && !hasPendingQueueItems) + { + removedOperation.Dispose(); + this.expandNodeQueue.CloseConnections(removedOperation.SqlConnection.DataSource, removedOperation.SqlConnection.Database, DefaultExpandTimeout); + } + else if (!removedOperation.FileTreeCreated) + { + removedOperation.Cancel(); + } + else if (hasPendingQueueItems) + { + this.expandNodeQueue.StopQueueProcessor(DefaultExpandTimeout); + } + } + await requestContext.SendResult(response); } #endregion + public void Dispose() + { + this.expandNodeQueue.Dispose(); + } + internal async Task RunFileBrowserOpenTask(FileBrowserOpenParams fileBrowserParams, RequestContext requestContext) { FileBrowserOpenedParams result = new FileBrowserOpenedParams(); + SqlConnection conn = null; + FileBrowserOperation browser = null; + bool isCancelRequested = false; + + if (this.expandNodeQueue.IsCancelRequested) + { + this.expandNodeQueue.StartQueueProcessor(); + } try { - ConnectionInfo connInfo; - this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo); - SqlConnection conn = null; - - if (connInfo != null) + if (!fileBrowserParams.ChangeFilter) { - DbConnection dbConn; - connInfo.TryGetConnection(ConnectionType.Default, out dbConn); - if (dbConn != null) + ConnectionInfo connInfo; + this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo); + if (connInfo != null) { - conn = ReliableConnectionHelper.GetAsSqlConnection(dbConn); + // Open new connection for each Open request + conn = ConnectionService.OpenSqlConnection(connInfo, this.serviceName); + browser = new FileBrowserOperation(conn, fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters); } } - - if (conn != null) - { - FileBrowserOperation browser = new FileBrowserOperation(conn, fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters); - browser.PopulateFileTree(); - - ownerToFileBrowserMap.AddOrUpdate(fileBrowserParams.OwnerUri, browser, (key, value) => browser); - - result.OwnerUri = fileBrowserParams.OwnerUri; - result.FileTree = browser.FileTree; - result.Succeeded = true; - } else { - result.Succeeded = false; + ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out browser); + if (browser != null) + { + browser.Initialize(fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters); + } + } + + if (browser != null) + { + ownerToFileBrowserMap.AddOrUpdate(fileBrowserParams.OwnerUri, browser, (key, value) => browser); + + // Create file browser tree + browser.PopulateFileTree(); + + // Check if cancel was requested + if (browser.IsCancellationRequested) + { + browser.Dispose(); + isCancelRequested = true; + } + else + { + result.OwnerUri = fileBrowserParams.OwnerUri; + result.FileTree = browser.FileTree; + result.Succeeded = true; + } } } catch (Exception ex) { - result.Succeeded = false; result.Message = ex.Message; } - await requestContext.SendEvent(FileBrowserOpenedNotification.Type, result); + if (!isCancelRequested) + { + await requestContext.SendEvent(FileBrowserOpenedNotification.Type, result); + } } internal async Task RunFileBrowserExpandTask(FileBrowserExpandParams fileBrowserParams, RequestContext requestContext) @@ -188,13 +235,35 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser FileBrowserExpandedParams result = new FileBrowserExpandedParams(); try { - FileBrowserOperation browser; - result.Succeeded = ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out browser); - if (result.Succeeded && browser != null) + FileBrowserOperation operation; + ConnectionInfo connInfo; + result.Succeeded = ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out operation); + this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo); + + if (result.Succeeded && operation != null && connInfo != null) { - result.Children = browser.GetChildren(fileBrowserParams.ExpandPath).ToArray(); - result.ExpandPath = fileBrowserParams.ExpandPath; - result.OwnerUri = fileBrowserParams.OwnerUri; + QueueItem queueItem = expandNodeQueue.QueueBindingOperation( + key: expandNodeQueue.AddConnectionContext(connInfo, this.serviceName), + bindingTimeout: DefaultExpandTimeout, + waitForLockTimeout: DefaultExpandTimeout, + bindOperation: (bindingContext, cancelToken) => + { + result.ExpandPath = fileBrowserParams.ExpandPath; + result.Children = operation.GetChildren(fileBrowserParams.ExpandPath).ToArray(); + result.OwnerUri = fileBrowserParams.OwnerUri; + return result; + }); + + queueItem.ItemProcessed.WaitOne(); + + if (this.expandNodeQueue.IsCancelRequested) + { + this.expandNodeQueue.CloseConnections(operation.SqlConnection.DataSource, operation.SqlConnection.Database, DefaultExpandTimeout); + } + else if (queueItem.GetResultAsT() != null) + { + result = queueItem.GetResultAsT(); + } } } catch (Exception ex) @@ -238,7 +307,6 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser } catch (Exception ex) { - result.Succeeded = false; result.Message = ex.Message; } diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs index feb0b302..62be60e3 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/BindingQueue.cs @@ -20,7 +20,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { internal const int QueueThreadStackSize = 5 * 1024 * 1024; - private CancellationTokenSource processQueueCancelToken = new CancellationTokenSource(); + private CancellationTokenSource processQueueCancelToken = null; private ManualResetEvent itemQueuedEvent = new ManualResetEvent(initialState: false); @@ -44,8 +44,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public BindingQueue() { this.BindingContextMap = new Dictionary(); + this.StartQueueProcessor(); + } - this.queueProcessorTask = StartQueueProcessor(); + public void StartQueueProcessor() + { + this.queueProcessorTask = StartQueueProcessorAsync(); } /// @@ -58,6 +62,18 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices return this.queueProcessorTask.Wait(timeout); } + /// + /// Returns true if cancellation is requested + /// + /// + public bool IsCancelRequested + { + get + { + return this.processQueueCancelToken.IsCancellationRequested; + } + } + /// /// Queue a binding request item /// @@ -182,7 +198,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } } - private bool HasPendingQueueItems + public bool HasPendingQueueItems { get { @@ -214,10 +230,16 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// /// Starts the queue processing thread /// - private Task StartQueueProcessor() + private Task StartQueueProcessorAsync() { + if (this.processQueueCancelToken != null) + { + this.processQueueCancelToken.Dispose(); + } + this.processQueueCancelToken = new CancellationTokenSource(); + return Task.Factory.StartNew( - ProcessQueue, + ProcessQueue, this.processQueueCancelToken.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); @@ -368,6 +390,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public void Dispose() { + if (this.processQueueCancelToken != null) + { + this.processQueueCancelToken.Dispose(); + } + if (itemQueuedEvent != null) { itemQueuedEvent.Dispose();