ClientModel: Shared HttpClient in PipelineTransport, and a couple more updates

sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs

@@ -100,9 +100,9 @@ public sealed override void Process(System.ClientModel.Primitives.PipelineMessag
         protected virtual bool ShouldRetryCore(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
         protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryCoreAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
         public void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        public System.Threading.Tasks.ValueTask WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        public System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
         protected virtual void WaitCore(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        protected virtual System.Threading.Tasks.ValueTask WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        protected virtual System.Threading.Tasks.Task WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
     public partial class HttpClientPipelineTransport : System.ClientModel.Primitives.PipelineTransport, System.IDisposable
     {

sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs

@@ -100,9 +100,9 @@ public sealed override void Process(System.ClientModel.Primitives.PipelineMessag
         protected virtual bool ShouldRetryCore(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
         protected virtual System.Threading.Tasks.ValueTask<bool> ShouldRetryCoreAsync(System.ClientModel.Primitives.PipelineMessage message, System.Exception? exception) { throw null; }
         public void Wait(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        public System.Threading.Tasks.ValueTask WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        public System.Threading.Tasks.Task WaitAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
         protected virtual void WaitCore(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { }
-        protected virtual System.Threading.Tasks.ValueTask WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
+        protected virtual System.Threading.Tasks.Task WaitCoreAsync(System.TimeSpan time, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
     public partial class HttpClientPipelineTransport : System.ClientModel.Primitives.PipelineTransport, System.IDisposable
     {

sdk/core/System.ClientModel/src/Message/BinaryContent.cs

@@ -114,7 +114,7 @@ private BinaryData Data
         {
             get
             {
-                if (_model is IJsonModel<T> && _options.Format == "J")
+                if (ModelReaderWriter.ShouldWriteAsJson(_model, _options))
                 {
                     throw new InvalidOperationException("Should use ModelWriter instead of _model.Write with IJsonModel.");
                 }
@@ -126,7 +126,7 @@ private BinaryData Data
 
         public override bool TryComputeLength(out long length)
         {
-            if (_model is IJsonModel<T> && _options.Format == "J")
+            if (ModelReaderWriter.ShouldWriteAsJson(_model, _options))
             {
                 return Writer.TryComputeLength(out length);
             }
@@ -142,7 +142,7 @@ public override bool TryComputeLength(out long length)
 
         public override void WriteTo(Stream stream, CancellationToken cancellation)
         {
-            if (_model is IJsonModel<T> && _options.Format == "J")
+            if (ModelReaderWriter.ShouldWriteAsJson(_model, _options))
             {
                 Writer.CopyTo(stream, cancellation);
                 return;
@@ -157,7 +157,7 @@ public override void WriteTo(Stream stream, CancellationToken cancellation)
 
         public override async Task WriteToAsync(Stream stream, CancellationToken cancellation)
         {
-            if (_model is IJsonModel<T> && _options.Format == "J")
+            if (ModelReaderWriter.ShouldWriteAsJson(_model, _options))
             {
                 await Writer.CopyToAsync(stream, cancellation).ConfigureAwait(false);
                 return;
@@ -176,4 +176,4 @@ public override void Dispose()
             }
         }
     }
-}
+}

sdk/core/System.ClientModel/src/ModelReaderWriter/ModelReaderWriter.cs

@@ -31,7 +31,7 @@ public static BinaryData Write<T>(T model, ModelReaderWriterOptions? options = d
 
         options ??= ModelReaderWriterOptions.Json;
 
-        if (IsJsonFormatRequested(model, options) && model is IJsonModel<T> jsonModel)
+        if (ShouldWriteAsJson(model, options, out IJsonModel<T> jsonModel))
         {
             using (ModelWriter<T> writer = new ModelWriter<T>(jsonModel, options))
             {
@@ -133,6 +133,21 @@ public static BinaryData Write(object model, ModelReaderWriterOptions? options =
         return GetInstance(returnType).Create(data, options);
     }
 
+    internal static bool ShouldWriteAsJson<T>(IPersistableModel<T> model, ModelReaderWriterOptions options)
+        => ShouldWriteAsJson(model, options, out _);
+
+    internal static bool ShouldWriteAsJson<T>(IPersistableModel<T> model, ModelReaderWriterOptions options, out IJsonModel<T> jsonModel)
+    {
+        if (IsJsonFormatRequested(model, options) && model is IJsonModel<T> iJsonModel)
+        {
+            jsonModel = iJsonModel;
+            return true;
+        }
+
+        jsonModel = default!;
+        return false;
+    }
+
     private static IPersistableModel<object> GetInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type returnType)
     {
         var model = GetObjectInstance(returnType) as IPersistableModel<object>;

sdk/core/System.ClientModel/src/Pipeline/ClientRetryPolicy.cs

@@ -182,10 +182,10 @@ protected virtual TimeSpan GetNextDelayCore(PipelineMessage message, int tryCoun
         return TimeSpan.FromMilliseconds((1 << (tryCount - 1)) * _initialDelay.TotalMilliseconds);
     }
 
-    public async ValueTask WaitAsync(TimeSpan time, CancellationToken cancellationToken)
+    public async Task WaitAsync(TimeSpan time, CancellationToken cancellationToken)
         => await WaitCoreAsync(time, cancellationToken).ConfigureAwait(false);
 
-    protected virtual async ValueTask WaitCoreAsync(TimeSpan time, CancellationToken cancellationToken)
+    protected virtual async Task WaitCoreAsync(TimeSpan time, CancellationToken cancellationToken)
     {
         await Task.Delay(time, cancellationToken).ConfigureAwait(false);
     }
@@ -244,4 +244,4 @@ private static bool IsRetriable(Exception exception)
             (exception is ClientResultException ex && ex.Status == 0);
 
     #endregion
-}
+}

sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.cs

@@ -16,6 +16,10 @@ public partial class HttpClientPipelineTransport : PipelineTransport, IDisposabl
     /// </summary>
     public static readonly HttpClientPipelineTransport Shared = new();
 
+    private static HttpClient? SharedDefaultClient;
+    private static object _lock = new object();
+    private static int _sharedClientRefCount;
+
     private readonly bool _ownsClient;
     private readonly HttpClient _httpClient;
 
@@ -39,6 +43,15 @@ public HttpClientPipelineTransport(HttpClient client)
 
     private static HttpClient CreateDefaultClient()
     {
+        lock (_lock)
+        {
+            if (SharedDefaultClient is not null)
+            {
+                _sharedClientRefCount++;
+                return SharedDefaultClient;
+            }
+        }
+
         // The following settings are added in Azure.Core and are not included
         // in System.ClientModel. If needed, we will migrate them into ClientModel.
         //   - SSL settings
@@ -52,11 +65,19 @@ private static HttpClient CreateDefaultClient()
 
         ServicePointHelpers.SetLimits(handler);
 
-        return new HttpClient(handler)
+        HttpClient defaultClient = new HttpClient(handler)
         {
             // Timeouts are handled by the pipeline
             Timeout = Timeout.InfiniteTimeSpan,
         };
+
+        lock (_lock)
+        {
+            _sharedClientRefCount++;
+            SharedDefaultClient = defaultClient;
+        }
+
+        return SharedDefaultClient;
     }
 
     protected override PipelineMessage CreateMessageCore()
@@ -196,13 +217,31 @@ protected virtual void Dispose(bool disposing)
         {
             if (this != Shared && _ownsClient)
             {
-                HttpClient httpClient = _httpClient;
-                httpClient?.Dispose();
+                if (_httpClient == SharedDefaultClient)
+                {
+                    lock (_lock)
+                    {
+                        _sharedClientRefCount--;
+
+                        if (_sharedClientRefCount == 0)
+                        {
+                            HttpClient httpClient = _httpClient;
+                            httpClient?.Dispose();
+
+                            SharedDefaultClient = null;
+                        }
+                    }
+                }
+                else
+                {
+                    HttpClient httpClient = _httpClient;
+                    httpClient?.Dispose();
+                }
             }
 
             _disposed = true;
         }
     }
 
     #endregion
-}
+}