From 9f5b1273e348ec20c1a1f0cc2f90b607b1e4d2ee Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 1 Jan 2026 17:09:38 +0000 Subject: [PATCH 1/4] fix: Don't dispose source stream by default in ToMemoryStreamAsync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1534 The ToMemoryStreamAsync extension method was unexpectedly disposing the input stream after copying, which is surprising behavior for an extension method. Additionally, behavior was inconsistent - MemoryStream inputs were returned as-is (not disposed), while other streams were disposed. Changes: - Add disposeSource parameter (defaults to false) - Reset output MemoryStream position to 0 for immediate reading - Also reset MemoryStream input position to 0 for consistency - Updated tests to pass disposeSource: true to prevent resource leaks Callers who want the old disposal behavior can pass disposeSource: true. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../Extensions/StreamExtensions.cs | 19 ++++++++++++++++--- test/ModularPipelines.UnitTests/FileTests.cs | 4 ++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/ModularPipelines/Extensions/StreamExtensions.cs b/src/ModularPipelines/Extensions/StreamExtensions.cs index 03104816f1..b01f0195b3 100644 --- a/src/ModularPipelines/Extensions/StreamExtensions.cs +++ b/src/ModularPipelines/Extensions/StreamExtensions.cs @@ -9,11 +9,20 @@ public static class StreamExtensions /// Turns a generic into a . /// /// Any stream. - /// A MemoryStream containing the Stream's data. - public static async Task ToMemoryStreamAsync(this Stream stream) + /// + /// When true, disposes the source stream after copying. + /// Defaults to false - callers are responsible for disposing the source stream. + /// + /// A MemoryStream containing the Stream's data, with Position set to 0 for reading. + public static async Task ToMemoryStreamAsync(this Stream stream, bool disposeSource = false) { if (stream is MemoryStream memoryStream) { + if (memoryStream.CanSeek) + { + memoryStream.Position = 0; + } + return memoryStream; } @@ -26,8 +35,12 @@ public static async Task ToMemoryStreamAsync(this Stream stream) await stream.CopyToAsync(memoryStream).ConfigureAwait(false); - await stream.DisposeAsync().ConfigureAwait(false); + if (disposeSource) + { + await stream.DisposeAsync().ConfigureAwait(false); + } + memoryStream.Position = 0; return memoryStream; } } \ No newline at end of file diff --git a/test/ModularPipelines.UnitTests/FileTests.cs b/test/ModularPipelines.UnitTests/FileTests.cs index 91979e3088..0a95bfa03f 100644 --- a/test/ModularPipelines.UnitTests/FileTests.cs +++ b/test/ModularPipelines.UnitTests/FileTests.cs @@ -107,7 +107,7 @@ public async Task ReadEmptyFile() var plainText = await file.ReadAsync(); var lines = await ToListAsync(file.ReadLinesAsync()); var bytes = await file.ReadBytesAsync(); - var stream = await file.GetStream().ToMemoryStreamAsync(); + var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); using (Assert.Multiple()) { @@ -128,7 +128,7 @@ public async Task ReadWriteFile() var plainText = await file.ReadAsync(); var lines = await ToListAsync(file.ReadLinesAsync()); var bytes = await file.ReadBytesAsync(); - var stream = await file.GetStream().ToMemoryStreamAsync(); + var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); using (Assert.Multiple()) { From 1e4f12974daec5a5014bc7b3f12dab2c84374ef5 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 1 Jan 2026 17:14:58 +0000 Subject: [PATCH 2/4] fix: Handle disposeSource consistently for MemoryStream inputs When disposeSource=true and input is a MemoryStream, now copies to a new MemoryStream before disposing the original. This ensures consistent behavior regardless of input stream type. Before: MemoryStream inputs were returned directly, ignoring disposeSource After: MemoryStream inputs are copied when disposeSource=true, then disposed --- .../Extensions/StreamExtensions.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ModularPipelines/Extensions/StreamExtensions.cs b/src/ModularPipelines/Extensions/StreamExtensions.cs index b01f0195b3..a6be8363f8 100644 --- a/src/ModularPipelines/Extensions/StreamExtensions.cs +++ b/src/ModularPipelines/Extensions/StreamExtensions.cs @@ -16,17 +16,21 @@ public static class StreamExtensions /// A MemoryStream containing the Stream's data, with Position set to 0 for reading. public static async Task ToMemoryStreamAsync(this Stream stream, bool disposeSource = false) { - if (stream is MemoryStream memoryStream) + // If input is already a MemoryStream and we don't need to dispose it, + // return it directly (optimization to avoid unnecessary copy) + if (stream is MemoryStream sourceMs && !disposeSource) { - if (memoryStream.CanSeek) + if (sourceMs.CanSeek) { - memoryStream.Position = 0; + sourceMs.Position = 0; } - return memoryStream; + return sourceMs; } - memoryStream = new MemoryStream(); + // Copy to new MemoryStream (handles both non-MemoryStream inputs + // and MemoryStream inputs when disposeSource is true) + var memoryStream = new MemoryStream(); if (stream.CanSeek) { From 3fae873cbf735ab1d8963770e6a2f5ac21afe32f Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 1 Jan 2026 17:29:15 +0000 Subject: [PATCH 3/4] fix: Use await using for MemoryStream in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use await using declaration for MemoryStream results in tests for consistency and to ensure proper disposal patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- test/ModularPipelines.UnitTests/FileTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ModularPipelines.UnitTests/FileTests.cs b/test/ModularPipelines.UnitTests/FileTests.cs index 0a95bfa03f..c99467362b 100644 --- a/test/ModularPipelines.UnitTests/FileTests.cs +++ b/test/ModularPipelines.UnitTests/FileTests.cs @@ -107,7 +107,7 @@ public async Task ReadEmptyFile() var plainText = await file.ReadAsync(); var lines = await ToListAsync(file.ReadLinesAsync()); var bytes = await file.ReadBytesAsync(); - var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); + await using var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); using (Assert.Multiple()) { @@ -128,7 +128,7 @@ public async Task ReadWriteFile() var plainText = await file.ReadAsync(); var lines = await ToListAsync(file.ReadLinesAsync()); var bytes = await file.ReadBytesAsync(); - var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); + await using var stream = await file.GetStream().ToMemoryStreamAsync(disposeSource: true); using (Assert.Multiple()) { From 896f78de97afdec2d1bc2c34ff0a56d00ea6fd7b Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 1 Jan 2026 17:41:54 +0000 Subject: [PATCH 4/4] docs: Add detailed remarks for ToMemoryStreamAsync breaking change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document: - Breaking change from previous behavior (now defaults to no disposal) - MemoryStream optimization behavior - Caller responsibility for disposing returned stream 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../Extensions/StreamExtensions.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/ModularPipelines/Extensions/StreamExtensions.cs b/src/ModularPipelines/Extensions/StreamExtensions.cs index a6be8363f8..9e28c67532 100644 --- a/src/ModularPipelines/Extensions/StreamExtensions.cs +++ b/src/ModularPipelines/Extensions/StreamExtensions.cs @@ -8,6 +8,21 @@ public static class StreamExtensions /// /// Turns a generic into a . /// + /// + /// + /// Breaking change: This method previously always disposed the source stream. + /// It now defaults to disposeSource: false to follow the principle of least surprise. + /// Existing callers that relied on automatic disposal should pass disposeSource: true. + /// + /// + /// When the input is already a and is false, + /// the same instance is returned (with position reset) as an optimization. + /// When is true, a copy is made before disposing the source. + /// + /// + /// Callers are responsible for disposing the returned . + /// + /// /// Any stream. /// /// When true, disposes the source stream after copying.