Close Stream when creating an ImageSource from a Uri

[bgrainger]

Fixes #6842

Description

During XAML serialization, an ImageSource may first be loaded by ImageSourceConverter.ConvertFrom. This calls BitmapDecoder.CreateFromUriOrStream with a pack URI, opens a Stream, and keeps it open. This will eventually be closed by the BitmapDecoder finalizer (see comments in that method).

https://github.com/dotnet/wpf/blob/c8742b5c39f72927e8598825291d6cf4e593df10/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Imaging/BitmapDecoder.cs#L215-L221

Subsequently the same pack URI is opened by ImageSourceTypeConverter.ConvertTo. This throws an IOException: 'Entries cannot be opened multiple times in Update mode.'. This occurs because the same ZipArchiveEntry is being opened a second time, while the first stream is still open.

By specifying BitmapCacheOption.OnLoad here, BitmapDecoder.Initialize will eagerly close the stream, which allows the ZipArchiveEntry to be opened a second time.

Customer Impact

Fixes an exception that prevents certain documents (presumably ones containing images, or ImageBrushes?) from printing.

Regression

Yes, regression from .NET Framework 4.8.

Testing

Tested in real-world application by using Faithlife.WPF: https://github.com/Faithlife/wpf/blob/stable/README.md#faithlifewpf

Risk

Unknown. May improve system resource usage by closing streams early instead of letting them be closed by a finalizer? OTOH, may perform unnecessary image decoding work up-front?

Microsoft Reviewers: Open in CodeFlow

[bgrainger]

The call stack for the first time the pack URI is opened (this is the stack addressed by this PR):

PresentationCore.dll!System.IO.Packaging.PackWebResponse.GetResponseStream() Line 179	C#
PresentationCore.dll!System.Windows.Media.Imaging.BitmapDecoder.SetupDecoderFromUriOrStream(System.Uri uri, System.IO.Stream stream, System.Windows.Media.Imaging.BitmapCacheOption cacheOption, out System.Guid clsId, out bool isOriginalWritable, out System.IO.Stream uriStream, out System.IO.UnmanagedMemoryStream unmanagedMemoryStream, out Microsoft.Win32.SafeHandles.SafeFileHandle safeFilehandle) Line 1056	C#
PresentationCore.dll!System.Windows.Media.Imaging.BitmapDecoder.CreateFromUriOrStream(System.Uri baseUri, System.Uri uri, System.IO.Stream stream, System.Windows.Media.Imaging.BitmapCreateOptions createOptions, System.Windows.Media.Imaging.BitmapCacheOption cacheOption, System.Net.Cache.RequestCachePolicy uriCachePolicy, bool insertInDecoderCache) Line 306	C#
PresentationCore.dll!System.Windows.Media.Imaging.BitmapFrame.CreateFromUriOrStream(System.Uri baseUri, System.Uri uri, System.IO.Stream stream, System.Windows.Media.Imaging.BitmapCreateOptions createOptions, System.Windows.Media.Imaging.BitmapCacheOption cacheOption, System.Net.Cache.RequestCachePolicy uriCachePolicy) Line 74	C#
PresentationCore.dll!System.Windows.Media.ImageSourceConverter.ConvertFrom(System.ComponentModel.ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value) Line 113	C#
System.Xaml.dll!MS.Internal.Xaml.Runtime.ClrObjectRuntime.CreateObjectWithTypeConverter(MS.Internal.Xaml.ServiceProviderContext serviceContext, System.Xaml.Schema.XamlValueConverter<System.ComponentModel.TypeConverter> ts, object value) Line 630	C#
System.Xaml.dll!MS.Internal.Xaml.Runtime.ClrObjectRuntime.CreateFromValue(MS.Internal.Xaml.ServiceProviderContext serviceContext, System.Xaml.Schema.XamlValueConverter<System.ComponentModel.TypeConverter> ts, object value, System.Xaml.XamlMember property) Line 144	C#
System.Xaml.dll!System.Xaml.XamlObjectWriter.Logic_CreateFromValue(MS.Internal.Xaml.Context.ObjectWriterContext ctx, System.Xaml.Schema.XamlValueConverter<System.ComponentModel.TypeConverter> typeConverter, object value, System.Xaml.XamlMember property, string targetName, MS.Internal.Xaml.Runtime.IAddLineInfo lineInfo) Line 1332	C#
System.Xaml.dll!System.Xaml.XamlObjectWriter.Logic_CreatePropertyValueFromValue(MS.Internal.Xaml.Context.ObjectWriterContext ctx) Line 1413	C#
System.Xaml.dll!System.Xaml.XamlObjectWriter.WriteEndMember() Line 803	C#
System.Xaml.dll!System.Xaml.XamlWriter.WriteNode(System.Xaml.XamlReader reader) Line 50	C#
PresentationFramework.dll!System.Windows.Markup.WpfXamlLoader.TransformNodes(System.Xaml.XamlReader xamlReader, System.Xaml.XamlObjectWriter xamlWriter, bool onlyLoadOneNode, bool skipJournaledProperties, bool shouldPassLineNumberInfo, System.Xaml.IXamlLineInfo xamlLineInfo, System.Xaml.IXamlLineInfoConsumer xamlLineInfoConsumer, MS.Internal.Xaml.Context.XamlContextStack<System.Windows.Markup.WpfXamlFrame> stack, System.Windows.Markup.IStyleConnector styleConnector) Line 267	C#
PresentationFramework.dll!System.Windows.Markup.WpfXamlLoader.Load(System.Xaml.XamlReader xamlReader, System.Xaml.IXamlObjectWriterFactory writerFactory, bool skipJournaledProperties, object rootObject, System.Xaml.XamlObjectWriterSettings settings, System.Uri baseUri) Line 148	C#
PresentationFramework.dll!System.Windows.Markup.WpfXamlLoader.Load(System.Xaml.XamlReader xamlReader, bool skipJournaledProperties, System.Uri baseUri) Line 24	C#
PresentationFramework.dll!System.Windows.Markup.XamlReader.Load(System.Xaml.XamlReader xamlReader, System.Windows.Markup.ParserContext parserContext) Line 964	C#
PresentationFramework.dll!System.Windows.Markup.XamlReader.Load(System.Xml.XmlReader reader, System.Windows.Markup.ParserContext parserContext, System.Windows.Markup.XamlParseMode parseMode, bool useRestrictiveXamlReader, System.Collections.Generic.List<System.Type> safeTypes) Line 920	C#
PresentationFramework.dll!System.Windows.Documents.XpsValidatingLoader.Load(System.IO.Stream stream, System.Uri parentUri, System.Windows.Markup.ParserContext pc, MS.Internal.ContentType mimeType, string rootElement) Line 67	C#
PresentationFramework.dll!System.Windows.Documents.PageContent._LoadPageImpl(System.Uri baseUri, System.Uri uriToLoad, out System.Windows.Documents.FixedPage fixedPage, out System.IO.Stream pageStream) Line 622	C#
PresentationFramework.dll!System.Windows.Documents.PageContent._LoadPage() Line 555	C#
PresentationFramework.dll!System.Windows.Documents.PageContent.GetPageRoot(bool forceReload) Line 108	C#
ReachFramework.dll!System.Windows.Xps.Serialization.ReachPageContentSerializerAsync.SerializePage(System.Windows.Xps.Serialization.SerializableObjectContext serializableObjectContext) Line 128	C#
ReachFramework.dll!System.Windows.Xps.Serialization.ReachPageContentSerializerAsync.AsyncOperation(System.Windows.Xps.Serialization.ReachSerializerContext context) Line 86	C#
ReachFramework.dll!System.Windows.Xps.Serialization.XpsOMSerializationManagerAsync.InvokeSaveAsXamlWorkItem(object arg) Line 148	C#

The call stack for the second time, which throws the IOException (see #6842):

PresentationCore.dll!System.IO.Packaging.PackWebResponse.GetResponseStream() Line 179	C#
PresentationCore.dll!MS.Internal.WpfWebRequestHelper.CreateRequestAndGetResponseStream(System.Uri uri) Line 180	C#
ReachFramework.dll!System.Windows.Xps.Serialization.ImageSourceTypeConverter.ConvertTo(System.ComponentModel.ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value, System.Type destinationType) Line 245	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualSerializer.WriteBitmap(string attribute, System.Windows.Media.ImageSource imageSource) Line 647	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualSerializer.BrushToString(System.Windows.Media.Brush brush, System.Windows.Rect bounds) Line 774	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualSerializer.FindBrush(System.Windows.Media.Brush brush, System.Windows.Rect bounds) Line 273	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualSerializer.WriteBrush(string attribute, System.Windows.Media.Brush brush, System.Windows.Rect bounds) Line 905	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualSerializer.System.Windows.Xps.Serialization.IMetroDrawingContext.DrawGeometry(System.Windows.Media.Brush brush, System.Windows.Media.Pen pen, System.Windows.Media.Geometry geometry) Line 1925	C#
ReachFramework.dll!System.Windows.Xps.Serialization.DrawingContextFlattener.DrawGeometry(System.Windows.Media.Brush brush, System.Windows.Media.Pen pen, System.Windows.Media.Geometry geometry) Line 283	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualTreeFlattener.DrawingWalk(System.Windows.Media.Drawing d, System.Windows.Media.Matrix drawingToWorldTransform) Line 699	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualTreeFlattener.DrawingWalk(System.Windows.Media.Drawing d, System.Windows.Media.Matrix drawingToWorldTransform) Line 761	C#
ReachFramework.dll!System.Windows.Xps.Serialization.VisualTreeFlattener.StartVisual(System.Windows.Media.Visual visual) Line 641	C#
ReachFramework.dll!System.Windows.Xps.Serialization.ReachVisualSerializerAsync.SerializeNextTreeNode(System.Windows.Xps.Serialization.ReachVisualSerializerContext context) Line 174	C#
ReachFramework.dll!System.Windows.Xps.Serialization.ReachVisualSerializerAsync.AsyncOperation(System.Windows.Xps.Serialization.ReachSerializerContext context) Line 73	C#
ReachFramework.dll!System.Windows.Xps.Serialization.XpsOMSerializationManagerAsync.InvokeSaveAsXamlWorkItem(object arg) Line 148	C#

[Symbai]

Seems low-risk

I am not sure about this one. Changing the behavior here has a massive impact on all WPF applications because it's used in hot path area. Testing it on a single application for a single purpose does not seem enough.

Also you say its a regression from .NET Framework. I've decompiled the .NET Framework GAC assembly and it also uses BitmapCacheOption.Default instead of BitmapCacheOption.OnLoad which means your change does not revert the state, it generates a completely new one which increases the risk by a lot. It means no one has tested it besides of you and you tested it on a single app only.

Decompilation taken from PresentationCore.dll Version 4.8.9032.0:

[bgrainger]

Changed my risk assessment to "Unknown" to better align with the two following sentences.

I will posit that leaving Streams (a managed object) to be closed by a finalizer is strongly against .NET best practices, and should be avoided if possible.

[bgrainger]

Ultimately I suspect this is related to the switch from WindowsBase to System.IO.Packaging for Package et al APIs: see https://github.com/dotnet/wpf/issues/2085 and linked issues. However, I don't know what the "best" fix is for this; perhaps much earlier in the process than what this PR addresses?

[dipeshmsft]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[singhashish-wpf]

/azp run

[azure-pipelines[bot]]

Azure Pipelines failed to run 1 pipeline(s).

[bgrainger]

This PR should also fix https://github.com/dotnet/wpf/issues/9418.