Preserve references for full-transfer object parameters
This commit is contained in:
parent
2422737865
commit
14072c5df6
7 changed files with 153 additions and 22 deletions
|
|
@ -50,11 +50,20 @@ public enum MarshalIn: Equatable, Sendable {
|
|||
case objectArrayToC
|
||||
/// Convert `[T]` of C-scalar values to a contiguous C buffer (`const T *`).
|
||||
case scalarArrayToC
|
||||
/// Access the underlying pointer of an object or interface wrapper.
|
||||
case objectPointer
|
||||
/// Access the underlying pointer of an object wrapper.
|
||||
/// - Parameter consumingRefFunction: The C ref function to call on the
|
||||
/// pointer before the call when the callee consumes a reference
|
||||
/// (`transfer-ownership="full"`), e.g. `"g_object_ref"` or
|
||||
/// `"gtk_expression_ref"`; `nil` for `transfer-ownership="none"` (and
|
||||
/// the degenerate `"container"`), where the pointer is only borrowed.
|
||||
/// The Swift wrapper keeps its own reference and unrefs in `deinit`, so
|
||||
/// a consumed reference MUST be balanced here or the wrapper
|
||||
/// over-releases (use-after-free).
|
||||
case objectPointer(consumingRefFunction: String?)
|
||||
/// Access the underlying pointer of an interface-typed wrapper (a
|
||||
/// protocol existential `any Foo`).
|
||||
case interfacePointer
|
||||
/// - Parameter consumingRefFunction: See `objectPointer`.
|
||||
case interfacePointer(consumingRefFunction: String?)
|
||||
/// Pass the `rawValue` of an enum (Int → C int).
|
||||
case enumRaw
|
||||
/// Pass the `rawValue` of a bitfield (UInt32 → C uint).
|
||||
|
|
|
|||
|
|
@ -911,7 +911,7 @@ private func signalReturnBridge(_ m: Mapping) -> SignalReturnBridge? {
|
|||
return .init(cType: "UnsafeMutablePointer<CChar>?", zero: "nil",
|
||||
needsResultVar: true,
|
||||
emit: { ["return g_strdup(\($0))"] })
|
||||
case .objectPointer, .interfacePointer:
|
||||
case .objectPointer(_), .interfacePointer(_):
|
||||
// transfer=full (`.objectWrap`/`.interfaceWrap(adopt:)`) means C takes
|
||||
// ownership of a reference, so add one; transfer=none
|
||||
// (`.objectRetain`) hands over a borrowed pointer unchanged.
|
||||
|
|
@ -1053,18 +1053,21 @@ private func renderWrapperExpr(for p: ParameterPlan, rawName: String, ownerIsInt
|
|||
// A signal parameter is a borrowed pointer (transfer none); wrapping
|
||||
// it must copy/ref so the wrapper owns an independent instance. Boxed
|
||||
// records without a GIR copy-function (e.g. GdkToplevelSize) have no
|
||||
// `init(retaining:)` (only emitted when one is known — see
|
||||
// `init(retaining:)` (only emitted when one is known - see
|
||||
// renderRecord); adopt the borrowed pointer via `takingOwnership:`
|
||||
// instead, matching the only initializer such records expose.
|
||||
if case .boxedWrap(_, let copyFn) = p.mapping.marshalOut, copyFn == nil {
|
||||
return "\(baseType)(takingOwnership: \(rawName))"
|
||||
}
|
||||
return "\(baseType)(retaining: \(rawName))"
|
||||
case .objectPointer:
|
||||
// Inbound signal parameters are borrowed pointers from C. The
|
||||
// `consumingRefFunction` payload describes the OUTBOUND call-argument
|
||||
// direction only and is deliberately ignored here.
|
||||
case .objectPointer(_):
|
||||
let isOptional = p.mapping.swiftType.hasSuffix("?")
|
||||
let baseType = isOptional ? String(p.mapping.swiftType.dropLast()) : p.mapping.swiftType
|
||||
return "\(baseType)(retaining: \(rawName))"
|
||||
case .interfacePointer:
|
||||
case .interfacePointer(_):
|
||||
let isOptional = p.mapping.swiftType.hasSuffix("?")
|
||||
let baseType = isOptional ? String(p.mapping.swiftType.dropLast()) : p.mapping.swiftType
|
||||
return "\(baseType)Ref(retaining: \(rawName))"
|
||||
|
|
@ -1742,10 +1745,10 @@ private func marshalCallArg(_ param: ParameterPlan) -> String {
|
|||
return param.swiftName
|
||||
case .stringArrayToC, .stringConstArrayToC, .stringConstElementArrayToC, .objectArrayToC, .scalarArrayToC:
|
||||
return param.swiftName
|
||||
case .objectPointer, .interfacePointer:
|
||||
return pointerArg(param)
|
||||
case .objectPointer(let consumingRefFunction), .interfacePointer(let consumingRefFunction):
|
||||
return pointerArg(param, consumingRefFunction: consumingRefFunction)
|
||||
case .boxedPointer:
|
||||
return pointerArg(param)
|
||||
return pointerArg(param, consumingRefFunction: nil)
|
||||
case .callbackBox(let scope, _):
|
||||
// The callback itself is passed as the C arg (Swift closures with
|
||||
// @convention(c) convert to C function pointers automatically).
|
||||
|
|
@ -1756,14 +1759,26 @@ private func marshalCallArg(_ param: ParameterPlan) -> String {
|
|||
}
|
||||
}
|
||||
|
||||
/// Generates the C argument for an object or boxed parameter, handling
|
||||
/// optional (nullable) types via `.map { _instancePointer($0.pointer) }`
|
||||
/// so the C function receives `nil` when the Swift wrapper is `nil`.
|
||||
private func pointerArg(_ param: ParameterPlan) -> String {
|
||||
if param.mapping.swiftType.hasSuffix("?") {
|
||||
return "\(param.swiftName).map { _instancePointer($0.pointer) }"
|
||||
/// Generates the C argument for an object, interface, or boxed parameter,
|
||||
/// handling optional (nullable) types via `.map { ... }` so the C function
|
||||
/// receives `nil` when the Swift wrapper is `nil`.
|
||||
///
|
||||
/// - Parameter consumingRefFunction: When non-`nil`, the parameter is
|
||||
/// `transfer-ownership="full"`: the callee consumes one reference, so this
|
||||
/// ref function is called on the pointer first. The Swift wrapper keeps its
|
||||
/// own reference and releases it in `deinit`; without this the wrapper
|
||||
/// over-releases a reference it no longer owns (use-after-free).
|
||||
private func pointerArg(_ param: ParameterPlan, consumingRefFunction: String? = nil) -> String {
|
||||
func arg(_ pointerExpr: String) -> String {
|
||||
guard let ref = consumingRefFunction else {
|
||||
return "_instancePointer(\(pointerExpr))"
|
||||
}
|
||||
return "_instancePointer(_rawPointer(\(ref)(_instancePointer(\(pointerExpr)))))"
|
||||
}
|
||||
return "_instancePointer(\(param.swiftName).pointer)"
|
||||
if param.mapping.swiftType.hasSuffix("?") {
|
||||
return "\(param.swiftName).map { \(arg("$0.pointer")) }"
|
||||
}
|
||||
return arg("\(param.swiftName).pointer")
|
||||
}
|
||||
|
||||
/// Generates the return expression for a callable.
|
||||
|
|
|
|||
|
|
@ -1223,7 +1223,7 @@ private func planCallable(
|
|||
detail: "return type '\(returnMap.swiftType)' is not yet generated (category: \(returnMap.category))"))
|
||||
}
|
||||
// Interfaces produce unsupported marshalOut (protocols can't be
|
||||
// constructed). Params still work via .objectPointer.
|
||||
// constructed). Params still work via pointer marshal-in cases.
|
||||
if case .unsupported(let reason) = returnMap.marshalOut {
|
||||
return .skip(SkipEntry(symbol: fullName, cIdentifier: cIdentifier,
|
||||
reason: .unknownType, detail: "return type: \(reason)"))
|
||||
|
|
|
|||
|
|
@ -369,8 +369,18 @@ private func mapTypeRef(
|
|||
// `"full"` (and the degenerate `"container"` case, which doesn't
|
||||
// apply to a bare object pointer) hands us an owned reference already,
|
||||
// so the wrapper just adopts it via `init(takingOwnership:)`.
|
||||
//
|
||||
// The in direction is the mirror image: a `"full"` in-parameter means
|
||||
// the CALLEE consumes one of our references, so the call site must add
|
||||
// one first (the wrapper still unrefs in `deinit`). The ref function is
|
||||
// the type's own - `g_object_ref` for ordinary GObjects, but
|
||||
// `gtk_expression_ref`/`gsk_render_node_ref` for classes rooting their
|
||||
// own fundamental hierarchy, where `g_object_ref` trips `G_IS_OBJECT`.
|
||||
let consumingRef = transfer == .full
|
||||
? context.registry.refUnrefFunctions(for: resolved.girName).ref
|
||||
: nil
|
||||
return Mapping(swiftType: swiftType, cSwiftType: "UnsafeMutableRawPointer?",
|
||||
marshalIn: .objectPointer,
|
||||
marshalIn: .objectPointer(consumingRefFunction: consumingRef),
|
||||
marshalOut: transfer == .full ? .objectWrap(sink: false) : .objectRetain,
|
||||
gvalue: GValueOps(typeMacro: "G_TYPE_OBJECT",
|
||||
getterSuffix: "object", setterSuffix: "object"),
|
||||
|
|
@ -385,7 +395,8 @@ private func mapTypeRef(
|
|||
// initializer to construct from a raw pointer.
|
||||
let swiftType = context.registry.swiftTypeName(for: resolved, in: context.currentModule)
|
||||
return Mapping(swiftType: swiftType, cSwiftType: "UnsafeMutableRawPointer?",
|
||||
marshalIn: .interfacePointer, marshalOut: .interfaceWrap(adopt: transfer == .full),
|
||||
marshalIn: .interfacePointer(consumingRefFunction: transfer == .full ? "g_object_ref" : nil),
|
||||
marshalOut: .interfaceWrap(adopt: transfer == .full),
|
||||
category: .needsClass)
|
||||
|
||||
case .enumeration:
|
||||
|
|
|
|||
|
|
@ -25,6 +25,24 @@ struct RendererCallableTests {
|
|||
let registry = TypeRegistry(repositories: ["GObject": gobject])
|
||||
return MapContext(registry: registry, currentModule: "GObject", currentNamespace: "GObject")
|
||||
}
|
||||
/// Builds a context with an ordinary GObject and a non-GObject
|
||||
/// fundamental class that declares its own ref function.
|
||||
func makeTransferContext() -> MapContext {
|
||||
let gobject = Repository(namespaces: [
|
||||
Namespace(
|
||||
name: "GObject", version: "2.0",
|
||||
classes: [
|
||||
Class(name: "Object", cType: "GObject", parent: nil,
|
||||
getTypeFunction: "g_object_get_type"),
|
||||
Class(name: "Expr", cType: "GExpr", parent: nil,
|
||||
getTypeFunction: "g_expr_get_type",
|
||||
refFunc: "g_expr_ref", unrefFunc: "g_expr_unref"),
|
||||
]
|
||||
)
|
||||
])
|
||||
let registry = TypeRegistry(repositories: ["GObject": gobject])
|
||||
return MapContext(registry: registry, currentModule: "GObject", currentNamespace: "GObject")
|
||||
}
|
||||
|
||||
func renderCallable(_ plan: CallablePlan) -> String {
|
||||
let module = ModulePlan(
|
||||
|
|
@ -88,6 +106,64 @@ struct RendererCallableTests {
|
|||
#expect(!body.contains("@_spi(SGTKInternal) public func getDefaultObject"))
|
||||
}
|
||||
|
||||
// MARK: - transfer-ownership="full" object in-parameters
|
||||
|
||||
@Test("transfer-full object in-param refs before the call; transfer-none does not")
|
||||
func transferFullObjectParamAddsRef() throws {
|
||||
let ctx = makeTransferContext()
|
||||
let borrowed = GlobalFunction(
|
||||
name: "set_thing", cIdentifier: "g_set_thing",
|
||||
parameters: [Parameter(name: "thing", type: .typeRef("Object", namespace: "GObject"),
|
||||
cType: "GObject*", transferOwnership: .none)],
|
||||
returnValue: ReturnValue(type: .void))
|
||||
let consumed = GlobalFunction(
|
||||
name: "take_thing", cIdentifier: "g_take_thing",
|
||||
parameters: [Parameter(name: "thing", type: .typeRef("Object", namespace: "GObject"),
|
||||
cType: "GObject*", transferOwnership: .full)],
|
||||
returnValue: ReturnValue(type: .void))
|
||||
guard case .success(let borrowedPlan) = planFunction(borrowed, context: ctx),
|
||||
case .success(let consumedPlan) = planFunction(consumed, context: ctx) else {
|
||||
Issue.record("expected both functions to plan successfully")
|
||||
return
|
||||
}
|
||||
#expect(renderCallable(borrowedPlan).contains(
|
||||
"g_set_thing(_instancePointer(thing.pointer))"))
|
||||
#expect(renderCallable(consumedPlan).contains(
|
||||
"g_take_thing(_instancePointer(_rawPointer(g_object_ref(_instancePointer(thing.pointer)))))"))
|
||||
}
|
||||
|
||||
@Test("transfer-full nullable object in-param refs inside the map closure")
|
||||
func transferFullNullableObjectParamRefsInsideMap() throws {
|
||||
let fn = GlobalFunction(
|
||||
name: "take_maybe", cIdentifier: "g_take_maybe",
|
||||
parameters: [Parameter(name: "thing", type: .typeRef("Object", namespace: "GObject"),
|
||||
cType: "GObject*", isNullable: true, transferOwnership: .full)],
|
||||
returnValue: ReturnValue(type: .void))
|
||||
guard case .success(let plan) = planFunction(fn, context: makeTransferContext()) else {
|
||||
Issue.record("expected g_take_maybe to plan successfully")
|
||||
return
|
||||
}
|
||||
#expect(renderCallable(plan).contains(
|
||||
"thing.map { _instancePointer(_rawPointer(g_object_ref(_instancePointer($0.pointer)))) }"))
|
||||
}
|
||||
|
||||
@Test("transfer-full param of a non-GObject fundamental uses its own ref function")
|
||||
func transferFullFundamentalUsesOwnRefFunction() throws {
|
||||
let fn = GlobalFunction(
|
||||
name: "take_expr", cIdentifier: "g_take_expr",
|
||||
parameters: [Parameter(name: "expr", type: .typeRef("Expr", namespace: "GObject"),
|
||||
cType: "GExpr*", transferOwnership: .full)],
|
||||
returnValue: ReturnValue(type: .void))
|
||||
guard case .success(let plan) = planFunction(fn, context: makeTransferContext()) else {
|
||||
Issue.record("expected g_take_expr to plan successfully")
|
||||
return
|
||||
}
|
||||
let body = renderCallable(plan)
|
||||
#expect(body.contains(
|
||||
"g_take_expr(_instancePointer(_rawPointer(g_expr_ref(_instancePointer(expr.pointer)))))"))
|
||||
#expect(!body.contains("g_object_ref"))
|
||||
}
|
||||
|
||||
// MARK: - C3: Out-param tuples
|
||||
|
||||
@Test("Single out-param with no Swift return becomes the out-param's type")
|
||||
|
|
|
|||
|
|
@ -305,7 +305,7 @@ struct TypeMapperTests {
|
|||
nullable: false, transfer: .none, context: ctx)
|
||||
#expect(mapping.swiftType == "Object")
|
||||
#expect(mapping.cSwiftType == "UnsafeMutableRawPointer?")
|
||||
#expect(mapping.marshalIn == .objectPointer)
|
||||
#expect(mapping.marshalIn == .objectPointer(consumingRefFunction: nil))
|
||||
#expect(mapping.marshalOut == .objectRetain)
|
||||
}
|
||||
|
||||
|
|
@ -314,6 +314,7 @@ struct TypeMapperTests {
|
|||
let ctx = makeContext()
|
||||
let mapping = try map(.typeRef("Object", namespace: "GObject"),
|
||||
nullable: false, transfer: .full, context: ctx)
|
||||
#expect(mapping.marshalIn == .objectPointer(consumingRefFunction: "g_object_ref"))
|
||||
#expect(mapping.marshalOut == .objectWrap(sink: false))
|
||||
}
|
||||
|
||||
|
|
@ -331,7 +332,7 @@ struct TypeMapperTests {
|
|||
let mapping = try map(.typeRef("TypePlugin", namespace: "GObject"),
|
||||
nullable: false, transfer: .none, context: ctx)
|
||||
#expect(mapping.swiftType == "TypePlugin")
|
||||
#expect(mapping.marshalIn == .interfacePointer)
|
||||
#expect(mapping.marshalIn == .interfacePointer(consumingRefFunction: nil))
|
||||
#expect(mapping.marshalOut == .interfaceWrap(adopt: false))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -52,4 +52,23 @@ struct GtkSmokeTests {
|
|||
#expect(adjustment.value == 7)
|
||||
#expect(adjustment.getValue() == 7)
|
||||
}
|
||||
|
||||
@Test("A transfer-full object parameter keeps the Swift wrapper's reference balanced")
|
||||
func transferFullParameterDoesNotOverRelease() throws {
|
||||
// `gtk_shortcut_set_action` is annotated `(transfer full)`: it consumes
|
||||
// one reference. The generated binding must add one before the call, or
|
||||
// the `NamedAction` wrapper's `deinit` releases a reference it no longer
|
||||
// owns, GTK finalizes the action while the shortcut still points at it,
|
||||
// and reading it back trips `g_object_unref: assertion 'G_IS_OBJECT
|
||||
// (object)' failed` plus a use-after-free.
|
||||
let shortcut = Shortcut(trigger: nil, action: nil)
|
||||
do {
|
||||
let action = NamedAction(name: "app.quit")
|
||||
shortcut.setAction(action: action)
|
||||
}
|
||||
// The wrapper is gone; the action must still be alive inside `shortcut`.
|
||||
let stored = shortcut.getAction()
|
||||
#expect(stored != nil)
|
||||
#expect(stored?.toString().contains("app.quit") == true)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue