Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net461.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public void Dispose() { }
public System.IO.Stream? ExtractResponseContent() { throw null; }
public void SetProperty(string name, object value) { }
public void SetProperty(System.Type type, object value) { }
public Azure.Response TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(string name, out object? value) { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net472.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public void Dispose() { }
public System.IO.Stream? ExtractResponseContent() { throw null; }
public void SetProperty(string name, object value) { }
public void SetProperty(System.Type type, object value) { }
public Azure.Response TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(string name, out object? value) { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net6.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public void Dispose() { }
public System.IO.Stream? ExtractResponseContent() { throw null; }
public void SetProperty(string name, object value) { }
public void SetProperty(System.Type type, object value) { }
public Azure.Response TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(string name, out object? value) { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public void Dispose() { }
public System.IO.Stream? ExtractResponseContent() { throw null; }
public void SetProperty(string name, object value) { }
public void SetProperty(System.Type type, object value) { }
public Azure.Response TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(string name, out object? value) { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
Expand Down
31 changes: 26 additions & 5 deletions sdk/core/Azure.Core/src/HttpMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public sealed class HttpMessage : IDisposable
{
private ArrayBackedPropertyBag<ulong, object> _propertyBag;
private Response? _response;
private bool _ownsResponse;
Comment thread
annelo-msft marked this conversation as resolved.
Outdated

/// <summary>
/// Creates a new instance of <see cref="HttpMessage"/>.
Expand All @@ -29,6 +30,7 @@ public HttpMessage(Request request, ResponseClassifier responseClassifier)
ResponseClassifier = responseClassifier;
BufferResponse = true;
_propertyBag = new ArrayBackedPropertyBag<ulong, object>();
_ownsResponse = true;
}

/// <summary>
Expand Down Expand Up @@ -177,7 +179,9 @@ public void SetProperty(Type type, object value) =>
_propertyBag.Set((ulong)type.TypeHandle.Value, value);

/// <summary>
/// Returns the response content stream and releases it ownership to the caller. After calling this methods using <see cref="Azure.Response.ContentStream"/> or <see cref="Azure.Response.Content"/> would result in exception.
/// Returns the response content stream and releases it ownership to the caller.
Comment thread
annelo-msft marked this conversation as resolved.
Outdated
/// After calling this methods using <see cref="Azure.Response.ContentStream"/>
/// or <see cref="Azure.Response.Content"/> would result in exception.
Comment thread
annelo-msft marked this conversation as resolved.
Outdated
/// </summary>
/// <returns>The content stream or null if response didn't have any.</returns>
public Stream? ExtractResponseContent()
Comment thread
annelo-msft marked this conversation as resolved.
Expand All @@ -194,6 +198,20 @@ public void SetProperty(Type type, object value) =>
}
}

/// <summary>
/// Transfers dispose ownership of the message's Response property to the
/// holder of the Response. After calling this method, the <see cref="Response"/>
/// property will still hold the response object, but when this object is
/// disposed, it will no longer dispose the response as well.
/// </summary>
public Response TransferResponseDisposeOwnership()
Comment thread
annelo-msft marked this conversation as resolved.
Outdated
{
Response response = Response;
_response = null;
Comment thread
annelo-msft marked this conversation as resolved.
Outdated
_ownsResponse = false;
return response;
}

/// <summary>
/// Disposes the request and response.
/// </summary>
Expand All @@ -202,11 +220,14 @@ public void Dispose()
Request.Dispose();
_propertyBag.Dispose();

var response = _response;
if (response != null)
if (_ownsResponse)
{
_response = null;
response.Dispose();
var response = _response;
if (response != null)
{
_response = null;
response.Dispose();
}
}
}

Expand Down
43 changes: 43 additions & 0 deletions sdk/core/Azure.Core/tests/HttpMessageTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Threading.Tasks;
using System.Threading;
using System;
using Azure.Core.Pipeline;
using Azure.Core.TestFramework;
using NUnit.Framework;
Expand Down Expand Up @@ -380,7 +383,47 @@ public void AppliesHandlerWithLastSetWinsSemantics()
Assert.IsTrue(message.ResponseClassifier.IsErrorResponse(message));
}

[Test]
public async Task UnbufferedStreamAccessibleAfterMessageDisposed()
{
byte[] serverBytes = new byte[1000];
new Random().NextBytes(serverBytes);

HttpPipeline pipeline = HttpPipelineBuilder.Build(new MockClientOptions { Retry = { NetworkTimeout = Timeout.InfiniteTimeSpan } });

using TestServer testServer = new(async context =>
{
await context.Response.Body.WriteAsync(serverBytes, 0, serverBytes.Length).ConfigureAwait(false);
});

Response response;
using (HttpMessage message = pipeline.CreateMessage())
{
message.Request.Uri.Reset(testServer.Address);
message.BufferResponse = false;

await pipeline.SendAsync(message, default).ConfigureAwait(false);

response = message.TransferResponseDisposeOwnership();
}

Assert.NotNull(response.ContentStream);
byte[] clientBytes = new byte[serverBytes.Length];
int readLength = 0;
while (readLength < serverBytes.Length)
{
readLength += await response.ContentStream.ReadAsync(clientBytes, 0, serverBytes.Length);
}

Assert.AreEqual(serverBytes.Length, readLength);
CollectionAssert.AreEqual(serverBytes, clientBytes);
}

#region Helpers
internal class MockClientOptions : ClientOptions
{
}

private class StatusCodeHandler : ResponseClassificationHandler
{
private readonly int _statusCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public void Apply(System.ClientModel.Primitives.RequestOptions options) { }
public void Dispose() { }
protected virtual void Dispose(bool disposing) { }
public void SetProperty(System.Type type, object value) { }
public System.ClientModel.Primitives.PipelineResponse TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
public abstract partial class PipelineMessageClassifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ public void Apply(System.ClientModel.Primitives.RequestOptions options) { }
public void Dispose() { }
protected virtual void Dispose(bool disposing) { }
public void SetProperty(System.Type type, object value) { }
public System.ClientModel.Primitives.PipelineResponse TransferResponseDisposeOwnership() { throw null; }
public bool TryGetProperty(System.Type type, out object? value) { throw null; }
}
public abstract partial class PipelineMessageClassifier
Expand Down
38 changes: 27 additions & 11 deletions sdk/core/System.ClientModel/src/Message/PipelineMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class PipelineMessage : IDisposable
{
private ArrayBackedPropertyBag<ulong, object> _propertyBag;
private bool _disposed;
private bool _ownsResponse;

protected internal PipelineMessage(PipelineRequest request)
{
Expand All @@ -20,6 +21,8 @@ protected internal PipelineMessage(PipelineRequest request)

BufferResponse = true;
ResponseClassifier = PipelineMessageClassifier.Default;

_ownsResponse = true;
}

public PipelineRequest Request { get; }
Expand Down Expand Up @@ -125,33 +128,46 @@ internal void AssertResponse()
}
}

public PipelineResponse TransferResponseDisposeOwnership()
{
AssertResponse();

PipelineResponse response = Response!;
Response = null;
_ownsResponse = false;
return response;
}

#region IDisposable

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
if (disposing && !_disposed)
{
var request = Request;
PipelineRequest request = Request;
Comment thread
annelo-msft marked this conversation as resolved.
request?.Dispose();

_propertyBag.Dispose();

var response = Response;
if (response != null)
if (_ownsResponse)
{
response.Dispose();
Response = null;
PipelineResponse? response = Response;
if (response != null)
{
response.Dispose();
Response = null;
}
}

_disposed = true;
}
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

#endregion
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using NUnit.Framework;
using System.ClientModel.Primitives;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core.TestFramework;
using ClientModel.Tests.Mocks;
using NUnit.Framework;
using SyncAsyncTestBase = ClientModel.Tests.SyncAsyncTestBase;

namespace System.ClientModel.Tests.Message;

public class PipelineMessageTests
public class PipelineMessageTests : SyncAsyncTestBase
{
public PipelineMessageTests(bool isAsync) : base(isAsync)
{
}

[Test]
public void ApplyAddsRequestHeaders()
{
Expand Down Expand Up @@ -111,6 +119,43 @@ public void TryGetTypeKeyedPropertyReturnsCorrectValues()
Assert.AreEqual(4444, ((T4)value!).Value);
}

[Test]
public async Task UnbufferedStreamAccessibleAfterMessageDisposed()
{
byte[] serverBytes = new byte[1000];
new Random().NextBytes(serverBytes);

ClientPipelineOptions options = new() { NetworkTimeout = Timeout.InfiniteTimeSpan };
ClientPipeline pipeline = ClientPipeline.Create(options);

using TestServer testServer = new(async context =>
{
await context.Response.Body.WriteAsync(serverBytes, 0, serverBytes.Length).ConfigureAwait(false);
});

PipelineResponse response;
using (PipelineMessage message = pipeline.CreateMessage())
{
message.Request.Uri = testServer.Address;
message.BufferResponse = false;

await pipeline.SendSyncOrAsync(message, IsAsync);

response = message.TransferResponseDisposeOwnership();
}

Assert.NotNull(response.ContentStream);
byte[] clientBytes = new byte[serverBytes.Length];
int readLength = 0;
while (readLength < serverBytes.Length)
{
readLength += await response.ContentStream!.ReadAsync(clientBytes, 0, serverBytes.Length);
}

Assert.AreEqual(serverBytes.Length, readLength);
CollectionAssert.AreEqual(serverBytes, clientBytes);
}

#region Helpers
private struct T1
{
Expand Down