Bug 59547 - Premature collection possible in bindings in MetalPerformanceShaders
Summary: Premature collection possible in bindings in MetalPerformanceShaders
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll (show other bugs)
Version: master
Hardware: PC Mac OS
: --- major
Target Milestone: xcode9.2
Assignee: Alex Soto [MSFT]
Depends on:
Reported: 2017-09-18 07:55 UTC by Rolf Bjarne Kvinge [MSFT]
Modified: 2017-11-28 23:10 UTC (History)
2 users (show)

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


Description Rolf Bjarne Kvinge [MSFT] 2017-09-18 07:55:04 UTC
See: https://github.com/xamarin/xamarin-macios/blob/1690ccbc991ab59835f76a15ef510cd182259915/src/MetalPerformanceShaders/MPSKernel.cs#L23-L43

The following code is unsafe, because the GC is free to move the 'values' array once we're out of the fixed statement:

    fixed (nfloat *ptr = values)
        return (IntPtr) ptr;

this means that the calling function now has pointer to memory that the GC can move at any time.
Comment 1 Rolf Bjarne Kvinge [MSFT] 2017-09-18 07:57:18 UTC
Raising priority and targeting the next release (15.6), since this will cause random crashes (or other random behavior) when it occurs, and it will be very tricky to track down.
Comment 2 Sebastien Pouliot 2017-11-22 22:46:49 UTC
The manual bindings for CIVector has the same code.
Comment 3 Sebastien Pouliot 2017-11-24 15:05:33 UTC
CIVector fixed in https://github.com/xamarin/xamarin-macios/pull/3038

@Alex since you're working on MPS can you apply the same pattern on MPSKernel ? thanks!
Comment 4 Alex Soto [MSFT] 2017-11-26 21:18:13 UTC
PR: https://github.com/xamarin/xamarin-macios/pull/3005
Comment 5 Sebastien Pouliot 2017-11-27 22:05:39 UTC
Switching milestone to `xcode9.2` since it enables the framework on macOS (and will likely be merged into 15.5)
Comment 6 Alex Soto [MSFT] 2017-11-28 23:10:27 UTC
Fixed in xamarin-macios/xcode9.2 @ https://github.com/xamarin/xamarin-macios/commit/87f9e239898c16a64de153e6fb631b7ad8fb3865

Note You need to log in before you can comment on or make changes to this bug.