Bug 2297 - CGLayer retaining Handle, causing resource leaks
Summary: CGLayer retaining Handle, causing resource leaks
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: 5.0
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2011-12-01 08:59 UTC by Philippe Monteil
Modified: 2011-12-03 03:49 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Philippe Monteil 2011-12-01 08:59:18 UTC
The following method made of a simple loop performing
the allocation / destruction of a CGLayer causes resource leaks: 

[DllImport ("/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics")]
private static extern void CGLayerRelease (IntPtr handle);
[DllImport ("/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics")]
private static extern void CGLayerRetain (IntPtr handle);
[DllImport ("/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics")]
private static extern SizeF CGLayerGetSize (IntPtr layer);
[DllImport ("/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics")]
private static extern IntPtr CGLayerGetContext (IntPtr layer);
[DllImport ("/System/Library/Frameworks/CoreGraphics.framework/CoreGraphics")]
private static extern IntPtr CGLayerCreateWithContext (IntPtr context, SizeF size, IntPtr dictionary);
        
        public static void Test()
        {
            int _im = 1000;
            int _size = 2000;
            int Width = _size;
            int Height = _size;
            for (int i = 0; i < _im; i++)
            {
                Console.WriteLine("Test[" + i + "] 1.300 _size=" + _size);
                
                CGColorSpace _pCGColorSpace = null;
                CGBitmapContext _context = null;
                CGLayer _pCGLayer = null;
                try
                {                               
                    _pCGColorSpace = CGColorSpace.CreateDeviceRGB();
                    
                    _context = new CGBitmapContext(IntPtr.Zero, Width, Height, 8, Width * 4, _pCGColorSpace, CGImageAlphaInfo.NoneSkipLast);
                    
                    SizeF _sizeF = new SizeF((float)Width,(float)Height);
                    
                    _pCGLayer = CGLayer.Create(_context, _sizeF);
                    
                    {
                        IntPtr _handle = _pCGLayer != null ? _pCGLayer.Handle : IntPtr.Zero;
                        Console.WriteLine("Test[" + i + "] _handle=" + _handle);
                    }
                    
                }
                finally
                {
                    if (_pCGLayer != null)
                    {
                        IntPtr _handle = _pCGLayer.Handle; 
                        _pCGLayer.Dispose();
                        
                        // Necessary to fully release _pCGLayer.Handle
                        CGLayerRelease(_handle);
                        
                        /*** BOOM
                        try
                        {
                            CGLayerRelease(_handle);
                        }
                        catch (Exception E)
                        {
                        }
                        finally
                        {
                        }
                        ***/
                        
                    }
                    if (_pCGColorSpace != null) _pCGColorSpace.Dispose();
                    if (_context != null) _context.Dispose();
                }
                
            }
        }

The problem lies in the way CGLayer.Create method and the CGLayer constructor operate,
as seen in the code

public static CGLayer Create (CGContext context, SizeF size)
{
 return new CGLayer (CGLayer.CGLayerCreateWithContext(context.Handle, size, IntPtr.Zero));
}

internal CGLayer (IntPtr handle)
		{
			if (handle == IntPtr.Zero)
			{
				throw new Exception ("Invalid parameters to layer creation");
			}
			this.handle = handle;
			CGLayer.CGLayerRetain (handle);
		}

The CGLayer constructor systematically increases the reference of its handle,
which is not necessary since it was directly obtained from a call to CGLayer.CGLayerCreateWithContext, who returned value is already owned by its receiver.

The same problem arises with MonoMac, for the same reason.

IMO, the way the ownership of the handles emitted by CoreGraphics
should be reviewed, made more simple and systematic.
A quick look at CGContext show similar problems...
Comment 1 Rolf Bjarne Kvinge [MSFT] 2011-12-01 15:59:12 UTC
This has been fixed a couple of weeks ago (but no released version has the fix yet - I believe it will be included in the upcoming 5.0.3 release).

I don't see any similar problems with CGContext, exactly where do you see it?
Comment 2 Philippe Monteil 2011-12-02 04:17:55 UTC
Thanks for the quick and positive answer: will the fix reflect in MonoMac as well?
	
The CGLayer exposes a Context property like: 
	
public CGContext Context
{
	get
	{
		return new CGContext (CGLayer.CGLayerGetContext (this.handle));
	}
}

which invokes the CGContext constructor:
	
public CGContext (IntPtr handle)
{
	if (handle == IntPtr.Zero)
	{
		throw new Exception ("Invalid parameters to context creation");
	}
	CGContext.CGContextRetain (handle); // <<<<<
	this.handle = handle;
}

I suppose that CGLayer.CGLayerGetContext returns a handle which 'reference count' has already been incremented,
and it seems that this 'reference count' is incremented once more by CGContext.CGContextRetain before beeing
stored in the 'handle' private member.

~CGContext ()
{
	this.Dispose (false);
}

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

protected virtual void Dispose (bool disposing)
{
	if (this.handle != IntPtr.Zero)
	{
		CGContext.CGContextRelease (this.handle);
		this.handle = IntPtr.Zero;
	}
}

also, the role of the 'owns' parameter in the following internal constructor is somewhat strange 

internal CGContext (IntPtr handle, bool owns)
{
	if (!owns)
	{
		CGContext.CGContextRetain (handle);
	}
	if (handle == IntPtr.Zero)
	{
		throw new Exception ("Invalid handle");
	}
	this.handle = handle;
}
	
in that it doesn't reflect in the way the Dispose method operates.

It seems that calling CGContext.CGContextRelease (this.handle); on an invalid handle, among others,
provokes a brutal end of the current process, and that there is no way of preventing such a crash.
A precise management of the ja-handles emitted by OSX/iOS is therefore crucial.
Comment 3 Rolf Bjarne Kvinge [MSFT] 2011-12-02 06:03:19 UTC
Yes, the fix is in MonoMac too (since the source code is shared).

CGLayerGetContext does not return a refcounted handle (in contrast to CGLayerCreateWithContext), so that is not a leak.

What 'owns' means is that when the CGContext ctor is called with owns=true, the caller is saying "I give you the ownership I have of this handle", so the CGContext ctor does not have to call CGContextRetain on the handle.

Releasing an invalid handle will of course kill the process (as in any other refcounting systems), since you're accessing freed memory.
Comment 4 Philippe Monteil 2011-12-02 07:35:13 UTC
MonoMac: fine!

<<CGLayerGetContext does not return a refcounted handle (in contrast to
<<CGLayerCreateWithContext), so that is not a leak.
Is that implied by the Get vs Create prefix?
The official Apple documentation
http://disanji.net/iOS_Doc/#documentation/GraphicsImaging/Reference/CGLayer/Reference/reference.html
explicityl mentions the need to release the returned value in the case of CGLayerCreateWithContext, but says nothing for CGLayerGetContext.

<<Releasing an invalid handle will of course kill the process (as in any other
<<refcounting systems), since you're accessing freed memory.
Which would make useful a mechanism memorizing the handles emitted by the CoreGraphics APIs along with their refcount and then filtering the calls to the various 'Release' functions.
The fact that there is no way of capturing this kind of error is quite frightening :-)!

Thanks again for your quick and detailed answers!
Comment 5 Rolf Bjarne Kvinge [MSFT] 2011-12-02 18:56:14 UTC
CGLayerGetContext/CGLayerCreateWithContext: these methods don't have a prefix that implies that they return refcounted variables (so in theory they don't). But then CGLayerCreateWithContext's documentation explicitly states that it does return a refcounted value.

There is no way MonoTouch can keep track of when handles are retained/released, since it does not have full control of the platform (objc can retain/release without MonoTouch ever knowing about it). And in any case you shouldn't need to deal with refcounting directly anyway with MonoTouch, it should be handled automatically for you.
Comment 6 Philippe Monteil 2011-12-03 03:49:31 UTC
My idea was to integrate some code in the CSharp classes wrapping the iOS/OSX APIs which would store the handles those APIs emit, memorize and check their reference count so as to detecdt resource leaks, and prevent API calls which would otherwise boom the process.