Fix unhandled ObjectDisposedException from upload progress callback

The Azure blob upload progress handler read fileStream.Length on every
tick. Progress<T> dispatches its handlers on the thread pool, so a queued
callback could run after UploadFileAsync had returned or thrown and the
`using` had already disposed the stream. get_Length() then threw
ObjectDisposedException on a thread-pool thread, outside the caller's
try/catch in IStorePackagedAPIExtensions, taking the whole process down
with exit code 1.

Capture the length once before the upload so the callback never touches
the stream, and guard the handler body so a late callback can never
terminate the process. The guard also covers the progress consumer
itself: the CLI passes a Spectre.Console ProgressTask, which can throw
once the progress display has been torn down.

Fixes #154

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4f872603-9e85-44d2-a627-788a2806492a

Author: azchohfi

MSStore.CLI.UnitTests/AzureBlobManagerUnitTests.cs

@@ -0,0 +1,95 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using MSStore.CLI.Services;
+
+namespace MSStore.CLI.UnitTests
+{
+    [TestClass]
+    public class AzureBlobManagerUnitTests
+    {
+        private sealed class RecordingProgress : IProgress<double>
+        {
+            public List<double> Reported { get; } = [];
+
+            public void Report(double value) => Reported.Add(value);
+        }
+
+        private sealed class ThrowingProgress : IProgress<double>
+        {
+            public void Report(double value) => throw new InvalidOperationException("Progress display already torn down.");
+        }
+
+        /// <summary>
+        /// Regression test for https://github.com/microsoft/msstore-cli/issues/154.
+        ///
+        /// The upload progress callback used to read fileStream.Length. Progress&lt;T&gt; dispatches its
+        /// handlers on the thread pool, so a queued callback could run after UploadFileAsync had returned or
+        /// thrown and the `using` had disposed the stream. That threw ObjectDisposedException on a
+        /// thread-pool thread, outside the caller's try/catch, and took the whole process down.
+        /// </summary>
+        [TestMethod]
+        public void CreateProgressCallbackDoesNotTouchTheFileStreamAfterItIsDisposed()
+        {
+            var path = Path.GetTempFileName();
+
+            try
+            {
+                File.WriteAllBytes(path, new byte[1000]);
+
+                var progress = new RecordingProgress();
+                Action<long> callback;
+
+                using (var fileStream = new FileStream(path, FileMode.Open, FileAccess.Read))
+                {
+                    callback = AzureBlobManager.CreateProgressCallback(fileStream.Length, progress);
+                }
+
+                // The stream is now disposed, exactly as it is when a late callback is dispatched.
+                callback(250);
+
+                Assert.AreSequenceEqual([25d], progress.Reported);
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+
+        [TestMethod]
+        public void CreateProgressCallbackReportsPercentage()
+        {
+            var progress = new RecordingProgress();
+            var callback = AzureBlobManager.CreateProgressCallback(200, progress);
+
+            callback(0);
+            callback(50);
+            callback(200);
+
+            Assert.AreSequenceEqual([0d, 25d, 100d], progress.Reported);
+        }
+
+        [TestMethod]
+        public void CreateProgressCallbackDoesNotReportForAnEmptyFile()
+        {
+            var progress = new RecordingProgress();
+            var callback = AzureBlobManager.CreateProgressCallback(0, progress);
+
+            // Must not divide by zero and push NaN/Infinity into the progress display.
+            callback(0);
+
+            Assert.IsEmpty(progress.Reported);
+        }
+
+        [TestMethod]
+        public void CreateProgressCallbackSwallowsExceptionsFromTheProgressConsumer()
+        {
+            // The CLI passes a Spectre.Console ProgressTask, which can throw once the progress display has
+            // been torn down. A late callback runs on a thread-pool thread, so anything escaping here would
+            // terminate the process.
+            var callback = AzureBlobManager.CreateProgressCallback(100, new ThrowingProgress());
+
+            callback(50);
+        }
+    }
+}

MSStore.CLI/Services/AzureBlobManager.cs

@@ -19,6 +19,11 @@ public async Task<string> UploadFileAsync(string blobUri, string localFilePath,
         {
             using var fileStream = new FileStream(localFilePath, FileMode.Open, FileAccess.Read);
 
+            // Capture the length up front. Progress<T> dispatches its handlers on the thread pool, so a
+            // callback can still run after this method has returned (or thrown) and fileStream has been
+            // disposed. The callback must therefore never touch fileStream.
+            var totalBytes = fileStream.Length;
+
             var blobClientOptions = new BlobClientOptions();
             blobClientOptions.Retry.NetworkTimeout = TimeSpan.FromSeconds(uploadTimeout);
             blobClientOptions.AddPolicy(new AddCorrelationIdHeaderPolicy(), HttpPipelinePosition.PerCall);
@@ -29,10 +34,7 @@ public async Task<string> UploadFileAsync(string blobUri, string localFilePath,
                 {
                     ContentType = "application/zip"
                 },
-                ProgressHandler = new Progress<long>(bytesTransferred =>
-                {
-                    progress.Report((double)bytesTransferred * 100 / fileStream.Length);
-                }),
+                ProgressHandler = new Progress<long>(CreateProgressCallback(totalBytes, progress)),
             };
 
             var response = await blobClient.UploadAsync(fileStream, blobUploadOptions, ct);
@@ -46,6 +48,25 @@ public async Task<string> UploadFileAsync(string blobUri, string localFilePath,
             }
         }
 
+        internal static Action<long> CreateProgressCallback(long totalBytes, IProgress<double> progress)
+        {
+            return bytesTransferred =>
+            {
+                try
+                {
+                    if (totalBytes > 0)
+                    {
+                        progress.Report((double)bytesTransferred * 100 / totalBytes);
+                    }
+                }
+                catch (Exception)
+                {
+                    // Progress reporting is best-effort. This runs on a thread-pool thread, outside the
+                    // caller's try/catch, so anything that escapes here would terminate the process.
+                }
+            };
+        }
+
         public class AddCorrelationIdHeaderPolicy() : HttpPipelineSynchronousPolicy
         {
             public override void OnSendingRequest(HttpMessage message)