-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Swift Bindings] Implement Swift native handle as SafeHandle
#3052
[Swift Bindings] Implement Swift native handle as SafeHandle
#3052
Conversation
src/Swift.Bindings/src/Emitter/StringEmitter/Handler/TypeHandler.cs
Outdated
Show resolved
Hide resolved
src/Swift.Bindings/src/Emitter/StringEmitter/Handler/TypeHandler.cs
Outdated
Show resolved
Hide resolved
src/Swift.Bindings/src/Emitter/StringEmitter/Handler/TypeHandler.cs
Outdated
Show resolved
Hide resolved
src/Swift.Bindings/src/Emitter/StringEmitter/Handler/TypeHandler.cs
Outdated
Show resolved
Hide resolved
…melab into swift-bindings/implement-safe-handlers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
src/Swift.Runtime/src/Swift/Runtime/InteropServices/SwiftMarshal.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I'd update the PayloadBuffer
as suggested in a comment and then LGTM.
Thanks!
Description
To ensure thread safety and delegate Swift handle release to the native handler,
SwiftHandle
implementsSafeHandle
for a native Swift handle. Frozen structs that require memory management contain aTypeBuffer
struct is passed at the P/Invoke boundary, while a correspondingSwiftHandle
performs C# reference counting for the type.Swift
frozen
structs that require memory management are projected as C# classes:Swift
non-frozen
structs that require memory management are projected as C# classes:When
Dispose
is called on these types,_payload.Dispose
is invoked, which calls theValueWitnessTable->Destroy
function to free native resource.Changes
Implemented
SwiftHandle
asSafeHandle
Added reference counting around P/Invoke calls using
DangerousAddRef
andDangerousRelease
SwiftHandle _payload
SwiftHandle
is passed at the P/Invoke boundary with internal retain/release; the .NET runtime automatically manages reference counting at P/Invoke boundaryUpdated
MarshalToSwift
andMarshalFromSwift
to reflect the newSwiftHandle
implementationMarshalToSwift
updated to check for buffer overrunsRemoved the explicit
Destroy
call from the type'sDispose
methodEnsured
SwiftHandle
release resources by calling_metadata.ValueWitnessTable->Destroy
only after all references are releasedUpdated manual bindings to reflect the SwiftHandle implementation
Out of scope
Structs projected as C# classes do not require explicit
Dispose
methods or finalizers, since the GC can callDispose
of theSafeHandle
when the object is collected. Projecting frozen value types as structs is a UX decision and will not be made in this PR.Fixes #3018 #2975 #2974 #2851