Skip to content

Commit d7d8d43

Browse files
committed
Some improvements on top of PythonFunction
1. Make PythonFunction a struct; 2. Added explicit deallocate method in case we want to avoid leak; 3. Use consuming because PyCFunction_New has refcount 1;
1 parent 03fd1c9 commit d7d8d43

File tree

2 files changed

+34
-29
lines changed

2 files changed

+34
-29
lines changed

PythonKit/Python.swift

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,58 +1531,66 @@ public struct PythonBytes : PythonConvertible, ConvertibleFromPython, Hashable {
15311531
///
15321532
/// Python.map(PythonFunction { x in x * 2 }, [10, 12, 14]) // [20, 24, 28]
15331533
///
1534-
final public class PythonFunction {
1534+
final class PyFunction {
1535+
private var callSwiftFunction: (_ argumentsTuple: PythonObject) throws -> PythonConvertible
1536+
init(_ callSwiftFunction: @escaping (_ argumentsTuple: PythonObject) throws -> PythonConvertible) {
1537+
self.callSwiftFunction = callSwiftFunction
1538+
}
1539+
func callAsFunction(_ argumentsTuple: PythonObject) throws -> PythonConvertible {
1540+
try callSwiftFunction(argumentsTuple)
1541+
}
1542+
}
1543+
1544+
public struct PythonFunction {
15351545
/// Called directly by the Python C API
1536-
private let callSwiftFunction: (_ argumentsTuple: PythonObject) throws -> PythonConvertible
1546+
private var function: Unmanaged<PyFunction>
15371547

15381548
public init(_ fn: @escaping (PythonObject) throws -> PythonConvertible) {
1539-
self.callSwiftFunction = { argumentsAsTuple in
1549+
let function = PyFunction { argumentsAsTuple in
15401550
return try fn(argumentsAsTuple[0])
15411551
}
1552+
self.function = Unmanaged.passRetained(function)
15421553
}
15431554

15441555
/// For cases where the Swift function should accept more (or less) than one parameter, accept an ordered array of all arguments instead
15451556
public init(_ fn: @escaping ([PythonObject]) throws -> PythonConvertible) {
1546-
self.callSwiftFunction = { argumentsAsTuple in
1557+
let function = PyFunction { argumentsAsTuple in
15471558
return try fn(argumentsAsTuple.map { $0 })
15481559
}
1560+
self.function = Unmanaged.passRetained(function)
1561+
}
1562+
1563+
// FIXME: Memory management issue:
1564+
// It is necessary to pass a retained reference to `PythonFunction` so that it
1565+
// outlives the `PyReference` of the PyCFunction we create below. If we don't,
1566+
// Python tries to access what then has become a garbage pointer when it cleans
1567+
// up the CFunction. This means the entire `PythonFunction` currently leaks.
1568+
public func deallocate() {
1569+
function.release()
15491570
}
15501571
}
15511572

15521573
extension PythonFunction : PythonConvertible {
15531574
public var pythonObject: PythonObject {
15541575
_ = Python // Ensure Python is initialized.
15551576

1556-
// FIXME: Memory management issue:
1557-
// It is necessary to pass a retained reference to `PythonFunction` so that it
1558-
// outlives the `PyReference` of the PyCFunction we create below. If we don't,
1559-
// Python tries to access what then has become a garbage pointer when it cleans
1560-
// up the CFunction. This means the entire `PythonFunction` currently leaks.
1561-
let selfPointer = Unmanaged.passRetained(self).toOpaque()
1577+
let funcPointer = function.toOpaque()
15621578

1563-
let fnPointer = PyCFunction_New(
1579+
let pyFuncPointer = PyCFunction_New(
15641580
PythonFunction.sharedMethodDefinition,
1565-
selfPointer
1581+
funcPointer
15661582
)
15671583

1568-
// FIXME: Another potential memory management issue.
1569-
// I can't see how to stop these functions from being prematurely
1570-
// garbage collected unless we untrack it from the Python GC.
1571-
PyObject_GC_UnTrack(fnPointer)
1572-
1573-
return PythonObject(fnPointer)
1584+
return PythonObject(consuming: pyFuncPointer)
15741585
}
15751586
}
15761587

15771588
// The pointers here technically constitute a leak, but no more than
15781589
// a static string or a static struct definition at top level.
15791590
fileprivate extension PythonFunction {
15801591
static let sharedFunctionName: UnsafePointer<Int8> = {
1581-
let name = "pythonkit_swift_function"
1582-
let cString = name.utf8CString
1583-
let copy = UnsafeMutableBufferPointer<Int8>.allocate(capacity: cString.count)
1584-
_ = copy.initialize(from: cString)
1585-
return UnsafePointer(copy.baseAddress!)
1592+
let name: StaticString = "pythonkit_swift_function"
1593+
return UnsafeRawPointer(name.utf8Start).assumingMemoryBound(to: Int8.self)
15861594
}()
15871595

15881596
static let sharedMethodDefinition: UnsafeMutablePointer<PyMethodDef> = {
@@ -1605,11 +1613,11 @@ fileprivate extension PythonFunction {
16051613
return nil
16061614
}
16071615

1608-
let `self` = Unmanaged<PythonFunction>.fromOpaque(selfPointer).takeUnretainedValue()
1616+
let function = Unmanaged<PyFunction>.fromOpaque(selfPointer).takeUnretainedValue()
16091617

16101618
do {
16111619
let argumentsAsTuple = PythonObject(consuming: argumentsPointer)
1612-
return try self.callSwiftFunction(argumentsAsTuple).ownedPyObject
1620+
return try function(argumentsAsTuple).ownedPyObject
16131621
} catch {
16141622
PythonFunction.setPythonError(swiftError: error)
16151623
return nil // This must only be `nil` if an exception has been set
@@ -1637,7 +1645,7 @@ fileprivate extension PythonFunction {
16371645
}
16381646
}
16391647

1640-
extension PythonObject : Error {}
1648+
extension PythonObject: Error {}
16411649

16421650
// From Python's C Headers:
16431651
struct PyMethodDef {

PythonKit/PythonLibrary+Symbols.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,6 @@ let PyRun_SimpleString: @convention(c) (PyCCharPointer) -> Void =
6060
let PyCFunction_New: @convention(c) (PyMethodDefPointer, UnsafeMutableRawPointer) -> PyObjectPointer =
6161
PythonLibrary.loadSymbol(name: "PyCFunction_New")
6262

63-
let PyObject_GC_UnTrack: @convention(c) (PyObjectPointer) -> Void =
64-
PythonLibrary.loadSymbol(name: "PyObject_GC_UnTrack")
65-
6663
let PyErr_SetString: @convention(c) (PyObjectPointer, UnsafePointer<CChar>?) -> Void =
6764
PythonLibrary.loadSymbol(name: "PyErr_SetString")
6865

0 commit comments

Comments
 (0)