Allow VSGitExt to be constructed and subscribed to from background thread #1506

jcansdale · 2018-02-23T11:12:48Z

This PR allows the VSGitExt service to be constructed and subscribed to from background thread. This is necessary to allow the package from Show current PR on status bar #1396 to load on a background thread (package option AllowsBackgroundLoading = true).

Notes on using Task.ContinueWith to guarantee ordering of RefreshActiveRepositories

I wanted to understand what I could have done to keep the ordering of RefreshActiveRepositories consistent when I moved it off the main thread to the thread pool. In the VSGitExt constructor, it was changed from RefreshActiveRepositories() to Task.Run(() => RefreshActiveRepositories).

The body of RefreshActiveRepositories can be changed to use sync, but this doesn't guarantee the order in which tasks start executing on the thread pool. For example:

        async Task Lock()
        {
            var refreshLock = new object();
            var writer = new StringWriter();
            var tasks = new List<Task>();

            for (int x = 0; x < 10; x++)
            {
                var y = x;
                tasks.Add(Task.Run(() =>
                {
                    lock (refreshLock)
                    {
                        writer.Write(y + ", ");
                    }
                }));
            }

            await Task.WhenAll(tasks);
            Console.WriteLine(writer);
        }

Might output: 0, 3, 4, 5, 6, 7, 8, 9, 1, 2,

Interestingly the TaskCreationOptions.PreferFairness flag doesn't seem to make any difference when starting tasks:

        async Task PreferFairness()
        {
            var refreshLock = new object();
            var writer = new StringWriter();
            var tasks = new List<Task>();

            for (int x = 0; x < 10; x++)
            {
                var y = x;
                tasks.Add(Task.Factory.StartNew(() =>
                {
                    lock (refreshLock)
                    {
                        writer.Write(y + ", ");
                    }
                }, TaskCreationOptions.PreferFairness));
            }

            await Task.WhenAll(tasks);
            Console.WriteLine(writer);
        }

Which outputs: 1, 3, 4, 5, 6, 7, 8, 9, 2, 0,

An alternative approach is to use ContinueWith:

        async Task ContinueWith()
        {
            var writer = new StringWriter();
            var task = Task.CompletedTask;

            for (int x = 0; x < 10; x++)
            {
                var y = x;
                task = task.ContinueWith(_ => writer.Write(y + ", "), TaskScheduler.Default);
            }

            await task;
            Console.WriteLine(writer);
        }

Which consistently outputs: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,

I prefer this approach because the intended ordering is explicit when the ContinueWith method is used.

What do you think?

Lock prevent tasks being executed at the same time but doesn't stop them being started out of order.

PendingTasks is used to execute tasks in order on the thread pool.

This will allow this service to be used by a package that is initializing on a background thread.

grokys · 2018-02-26T16:28:22Z