Bug 21043 - CGDataProvider does not hold a reference to buffer, which can then be garbage collected
Summary: CGDataProvider does not hold a reference to buffer, which can then be garbage...
Status: VERIFIED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: master
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2014-07-02 10:04 UTC by Peter Golde
Modified: 2015-04-09 02:48 UTC (History)
4 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:
VERIFIED FIXED

Description Peter Golde 2014-07-02 10:04:16 UTC
The CGDataProvider(byte[] buffer, int offset, int count) constructor does not hold onto a reference to buffer. Thus, buffer can be garbage collected before the data provider is used to construct an image. The CGDataProvider instance should hold a reference to buffer so that it can be used safely and the Monotouch garbage collector doesn't free the memory prematurely.
Comment 1 Sebastien Pouliot 2014-07-02 15:15:50 UTC
There is a reference kept to `buffer` when you use:

   public CGDataProvider (byte [] buffer, int offset, int count)

so the GC should not collect it unless the `CGDataProvider` instance is disposed.


a. Can you provide a test case that shows your issue ?

b. Also include all your version informations*


* The easiest way to get exact version information is to use the "Xamarin Studio" menu, "About Xamarin Studio" item, "Show Details" button and copy/paste the version informations (you can use the "Copy Information" button).
Comment 2 Peter Golde 2014-07-02 16:41:27 UTC
After further debugging, retaining an instance doesn't seem to fix the problem.

The problem only reproduces using the SGen garbage collector. 

As far as I can tell, if the array is recently allocated, it resides in generation 0. If a collection occurs before the CGDataProvider is used, the array is copied in memory to generation 1, and the native CGDataProvider ends up referencing garbage memory because it doesn't know that the memory has moved.

Looking at the source code for CGDataProvider, it doesn't seem like there is any code that prevents the array from moving (or copies the data) when a garbage collection occurs.

I don't have a good small test case that I can attach at this time. It is a bit difficult to reproduce the issue.

Version Info:

=== Xamarin Studio ===

Version 5.1 (build 479)
Installation UUID: a0248109-ee04-4fb3-bea3-55b92bef282e
Runtime:
	Mono 3.4.0 ((no/954ed3c)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 304000214

=== Xamarin.Android ===

Not Installed

=== Apple Developer Tools ===

Xcode 5.1.1 (5085)
Build 5B1008

=== Xamarin.iOS ===

Version: 7.2.5.4 (Indie Edition)
Hash: e7f4f92
Branch: 
Build date: 2014-06-18 02:37:27-0400

=== Xamarin.Mac ===

Version:

=== Build Information ===

Release ID: 501000479
Git revision: e5a428cec75d4cc7e6e3ccd8192a3660d013e7dc
Build date: 2014-06-26 09:28:17-04
Xamarin addins: 190d93e026e17280e75a3680ef38f2630b9228d3

=== Operating System ===

Mac OS X 10.9.3
Darwin Peters-MacBook-Pro.local 13.2.0 Darwin Kernel Version 13.2.0
    Thu Apr 17 23:03:13 PDT 2014
    root:xnu-2422.100.13~1/RELEASE_X86_64 x86_64
Comment 3 Sebastien Pouliot 2014-07-02 17:39:49 UTC
You're right. The existing `fixed` statement only pin the memory for the duration of the call (p/invoke).

OTOH that pointer can be used later (by native code) and the corresponding managed memory could have moved (when sgen is used). That will need a `GCHandle.Alloc(buffer, GCHandleType.Pinned);`
Comment 4 Sebastien Pouliot 2014-07-02 20:41:34 UTC
Fixed in master / c16c3a72107ac5be9c1da73cc44cb6ff5657d9c7
Comment 5 Ram Chandra 2014-08-06 06:25:09 UTC
I have checked this issue on following:

Mac OS X 10.9.3
Xamarin Studio: 5.3 (build 413)
Xamarin.iOS: 7.4.0.89
Mono 3.8.0 ((no/9ffca33)
Xcode 5.1 

Build Information
Release ID: 503000413
Git revision: 0b0a22314801b920383045d46a578c716f55e481
Build date: 2014-08-06 04:53:22-04
Xamarin addins: 57e379e6a9454a1c0d97aa3f770e7ae7cc16c522

I have checked the definition of "CGDataProvider" class in assembly browser and observed that the "this.gch=GCHandle.Alloc(buffer, GCHandleType.Pinned);" statement is added to the "CGDataProvider (byte[] buffer, int offset, int count)". Now "CGDataProvider" hold reference to the buffer.

Sceencast: http://www.screencast.com/t/FDQICxdfK3

This issue has been fixed. Hence closing  this issue.