[Bgen] Fix bug in trampolines related to GC.KeepAlive issues. #22445
Conversation
he Generator writes the following code for trampolines:
Trampolines with no return value:
```csharp
[Preserve (Conditional = true)]
[UnmanagedCallersOnly]
[UserDelegateType (typeof (global::ARKit.GetGeolocationCallback))]
internal static unsafe void Invoke (IntPtr block, global::CoreLocation.CLLocationCoordinate2D coordinate, double altitude, NativeHandle error) {
var del = BlockLiteral.GetTarget<global::ARKit.GetGeolocationCallback> (block);
if (del is not null) {
del (coordinate, altitude, Runtime.GetNSObject<NSError> (error)!);
}
}
```
Trampolines with an enumerator that is not a smart enumerator:
```csharp
unsafe global::AudioToolbox.AudioBuffers Invoke (uint frameCount)
{
var ret = new global::AudioToolbox.AudioBuffers (invoker (BlockPointer, frameCount), false);
return ret!;
}
```
Trampolines with a return value that is a smart enumerator:
```csharp
unsafe global::AVFoundation.AVAudioEngineManualRenderingStatus Invoke (uint numberOfFrames, global::AudioToolbox.AudioBuffers outBuffer, ref int outError)
{
var outBuffer__handle__ = outBuffer.GetHandle ();
var ret = (AVAudioEngineManualRenderingStatus) (long) invoker (BlockPointer, numberOfFrames, outBuffer__handle__, (int*) global::System.Runtime.CompilerServices.Unsafe.AsPointer<int> (ref outError));
GC.KeepAlive (outBuffer);
return ret!;
}
```
Trampolines with a return value that is a value type and is not an enum or smart enum
```csharp
[UserDelegateType (typeof (global::AVFoundation.AVAudioSourceNodeRenderHandlerRaw))]
internal static unsafe int Invoke (IntPtr block, nint isSilence, nint timestamp, uint frameCount, nint outputData) {
var del = BlockLiteral.GetTarget<global::AVFoundation.AVAudioSourceNodeRenderHandlerRaw> (block);
var retval = del (isSilence, timestamp, frameCount, outputData);
return retval;
}
```
Trampolines with a return value that is a managed representation of an unmanaged resource:
```csharp
internal static unsafe NativeHandle Invoke (IntPtr block, uint inNumberOfPackets, global::AVFoundation.AVAudioConverterInputStatus* outStatus) {
var del = BlockLiteral.GetTarget<global::AVFoundation.AVAudioConverterInputHandler> (block);
var retval = del (inNumberOfPackets, out global::System.Runtime.CompilerServices.Unsafe.AsRef<global::AVFoundation.AVAudioConverterInputStatus> (outStatus));
return retval.GetHandle ();
}
```
In the case of the trampolines with a return value that is a managed representation of an unmanaged resource have a bug on how the return value is handled. The trampoline does the following:
```csharp
return retval.GetHandle ();
```
The bug resides in the fact that the C# GC can collect the managed resource, that will result in the handle reference count being reduced and could
result in the handler being collected in the middle of the trampoline call. To fix this we change the trampoline generator to use:
```csharp
return Runtime.RetainAndAutoreleaseNSObject (retval);
```
for NSObject and its native version for INativeObjects:
```csharp
return Runtime.RetainAndAutoreleaseNativeObject (retval);
```
Up until now we have been very lucky that we were using the GetHandle
call and that it is present both in INativeObjects and NSObjecets, yet
that was a mistake.
If you notice there are some small changes in the bindings API, that is
becuase we need to mark those interfaces that inherit from
INativeObjects as INativeObject in the api-definiton dlls otherwise the
generator will not be able to tell that they are INativeObject.
I have just fixed the interfaces that are needed for the trampolines, we
might need to create a different PR in which we find all interfaces and
fix the inheritance.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #2b4d2b5] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
✅ API diff for current PR / commit.NET ( No breaking changes )❗ API diff vs stable (Breaking changes).NET ( ❗ Breaking changes ❗ )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
💻 [CI Build #2b4d2b5] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #2b4d2b5] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #2b4d2b5] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #2b4d2b5] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 117 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
🔥 [CI Build #73a79dd] Build failed (Build macOS tests) 🔥Build failed for the job 'Build macOS tests' (with job status 'Failed') Pipeline on Agent |
🔥 [CI Build #73a79dd] Build failed (Build packages) 🔥Build failed for the job 'Build packages' (with job status 'Failed') Pipeline on Agent |
🔥 [PR Build #73a79dd] Build failed (Detect API changes) 🔥Build failed for the job 'Detect API changes' (with job status 'Failed') Pipeline on Agent |
he Generator writes the following code for trampolines:
Trampolines with no return value:
Trampolines with an enumerator that is not a smart enumerator:
Trampolines with a return value that is a smart enumerator:
Trampolines with a return value that is a value type and is not an enum or smart enum
Trampolines with a return value that is a managed representation of an unmanaged resource:
In the case of the trampolines with a return value that is a managed representation of an unmanaged resource have a bug on how the return value is handled. The trampoline does the following:
The bug resides in the fact that the C# GC can collect the managed resource, that will result in the handle reference count being reduced and could result in the handler being collected in the middle of the trampoline call. To fix this we change the trampoline generator to use:
for NSObject and its native version for INativeObjects:
Up until now we have been very lucky that we were using the GetHandle call and that it is present both in INativeObjects and NSObjecets, yet that was a mistake.
If you notice there are some small changes in the bindings API, that is becuase we need to mark those interfaces that inherit from INativeObjects as INativeObject in the api-definiton dlls otherwise the generator will not be able to tell that they are INativeObject.
I have just fixed the interfaces that are needed for the trampolines, we might need to create a different PR in which we find all interfaces and fix the inheritance.