diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Unix.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Unix.cs index 271da443ce0827..a8492d7ffcab7b 100644 --- a/src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Unix.cs +++ b/src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Unix.cs @@ -50,9 +50,18 @@ public sealed partial class Graphics : MarshalByRefObject, IDisposable, IDeviceC private bool disposed; private static float defDpiX; private static float defDpiY; + private Metafile.MetafileHolder? _metafileHolder; internal Graphics(IntPtr nativeGraphics) => NativeGraphics = nativeGraphics; + internal Graphics(IntPtr nativeGraphics, Image image) : this(nativeGraphics) + { + if (image is Metafile mf) + { + _metafileHolder = mf.AddMetafileHolder(); + } + } + ~Graphics() { Dispose(); @@ -226,6 +235,14 @@ public void Dispose() status = Gdip.GdipDeleteGraphics(new HandleRef(this, NativeGraphics)); NativeGraphics = IntPtr.Zero; Gdip.CheckStatus(status); + + if (_metafileHolder != null) + { + var mh = _metafileHolder; + _metafileHolder = null; + mh.GraphicsDisposed(); + } + disposed = true; } @@ -488,7 +505,7 @@ public static Graphics FromImage(Image image) int status = Gdip.GdipGetImageGraphicsContext(image.nativeImage, out graphics); Gdip.CheckStatus(status); - Graphics result = new Graphics(graphics); + Graphics result = new Graphics(graphics, image); Rectangle rect = new Rectangle(0, 0, image.Width, image.Height); Gdip.GdipSetVisibleClip_linux(result.NativeGraphics, ref rect); diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.Unix.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.Unix.cs index 53aaca69fd4be3..361565d3f16642 100644 --- a/src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.Unix.cs +++ b/src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.Unix.cs @@ -35,6 +35,7 @@ using System.IO; using System.Reflection; using System.ComponentModel; +using System.Diagnostics; using System.Runtime.InteropServices; using Gdip = System.Drawing.SafeNativeMethods.Gdip; using System.Runtime.Serialization; @@ -43,6 +44,93 @@ namespace System.Drawing.Imaging { public sealed partial class Metafile : Image { + // Non-null if a graphics instance was created using + // Graphics.FromImage(this) The metadata holder is responsible for + // freeing the nativeImage if the Metadata instance is disposed before + // the Graphics instance. + private MetafileHolder? _metafileHolder; + + // A class responsible for disposing of the native Metafile instance + // if it needs to outlive the managed Metafile instance. + // + // The following are both legal with win32 GDI+: + // Metafile mf = ...; // get a metafile instance + // Graphics g = Graphics.FromImage(mf); // get a graphics instance + // g.Dispose(); mf.Dispose(); // dispose of the graphics instance first + // OR + // mf.Dispose(); g.Dispose(); // dispose of the metafile instance first + // + // ligbgdiplus has a bug where disposing of the metafile instance first will + // trigger a use of freed memory when the graphics instance is disposed, which + // could lead to crashes when the native memory is reused. + // + // The metafile holder is designed to take ownership of the native metafile image + // when the managed Metafile instance is disposed while a Graphics instance is still + // not disposed (ie the second code pattern above) and to keep the native image alive until the graphics + // instance is disposed. + // + // Note that the following throws, so we only ever need to keep track of one Graphics + // instance at a time: + // Metafile mf = ...; // get a metafile instance + // Graphics g = Graphics.FromImage(mf); + // Graphics g2 = Graphics.FromImage(mf); // throws OutOfMemoryException on GDI+ on Win32 + internal sealed class MetafileHolder : IDisposable + { + private bool _disposed; + private IntPtr _nativeImage; + + + internal bool Disposed { get => _disposed; } + internal MetafileHolder() + { + _disposed = false; + _nativeImage = IntPtr.Zero; + } + + ~MetafileHolder() => Dispose(false); + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + internal void Dispose(bool disposing) + { + if (!_disposed) + { + IntPtr nativeImage = _nativeImage; + _nativeImage = IntPtr.Zero; + _disposed = true; + if (nativeImage != IntPtr.Zero) + { + int status = Gdip.GdipDisposeImage(nativeImage); + Gdip.CheckStatus(status); + } + } + } + + internal void MetafileDisposed(IntPtr nativeImage) + { + _nativeImage = nativeImage; + } + + internal void GraphicsDisposed() + { + Dispose(); + } + } + + internal MetafileHolder? AddMetafileHolder() + { + // If _metafileHolder is not null and hasn't been disposed yet, there's already a graphics instance associated with + // this metafile, the native code will return an error status. + if (_metafileHolder != null && !_metafileHolder.Disposed) + return null; + _metafileHolder = new MetafileHolder(); + return _metafileHolder; + } + // Usually called when cloning images that need to have // not only the handle saved, but also the underlying stream // (when using MS GDI+ and IStream we must ensure the stream stays alive for all the life of the Image) @@ -143,6 +231,21 @@ public Metafile(string fileName, IntPtr referenceHdc, Rectangle frameRect, Metaf Gdip.CheckStatus(status); } + protected override void Dispose(bool disposing) + { + if (_metafileHolder != null && !_metafileHolder.Disposed) + { + // There's a graphics instance created from this Metafile, + // transfer responsibility for disposing the nativeImage to the + // MetafileHolder + _metafileHolder.MetafileDisposed(nativeImage); + _metafileHolder = null; + nativeImage = IntPtr.Zero; + } + + base.Dispose(disposing); + } + // methods public IntPtr GetHenhmetafile()