diff --git a/modules/packages/Python.chpl b/modules/packages/Python.chpl index 409539cf897d..42b2cf774ed7 100644 --- a/modules/packages/Python.chpl +++ b/modules/packages/Python.chpl @@ -246,7 +246,6 @@ module Python { var sepPy = PyObject_GetAttrString(os, "pathsep"); interp.checkException(); - defer Py_DECREF(sepPy); var sep = interp.fromPython(string, sepPy); return sep; @@ -265,8 +264,6 @@ module Python { @chpldoc.nodoc var converters: List.list(owned TypeConverter); @chpldoc.nodoc - var toFree: List.list(PyObjectPtr); - @chpldoc.nodoc var objgraph: PyObjectPtr = nil; @chpldoc.nodoc @@ -353,10 +350,6 @@ module Python { @chpldoc.nodoc proc deinit() { - for obj in this.toFree { - Py_CLEAR(c_ptrTo(obj)); - } - if pyMemLeaks && this.objgraph != nil { // note: try! is used since we can't have a throwing deinit @@ -464,6 +457,8 @@ module Python { /* Convert a Chapel value to a python object. This clones the Chapel value. + This returns a new reference to a Python object. + .. note:: Most users should not need to call this directly, except when writing @@ -507,6 +502,7 @@ module Python { } else if isSubtype(t, List.list) { return toList(val); } else if isSubtype(t, Value) { + Py_INCREF(val.get()); return val.get(); } else if t == NoneType { return Py_None; @@ -518,6 +514,10 @@ module Python { /* Convert a Python object to a Chapel value. This clones the Python value. + This steals a reference to the Python object, so the Chapel object will + either own the Python object or it will decrement the reference count + when it is done with it. + .. note:: Most users should not need to call this directly, except when writing @@ -544,12 +544,9 @@ module Python { return v: t; // cast to the real type } else if isBoolType(t) { return if obj == Py_True then true else false; - } else if t == c_ptrConst(c_char) { - var v = PyUnicode_AsUTF8(obj); - this.checkException(); - return v; } else if t == string { var v = string.createCopyingBuffer(PyUnicode_AsUTF8(obj)); + Py_DECREF(obj); this.checkException(); return v; } else if isArrayType(t) { @@ -571,6 +568,9 @@ module Python { } else if isSubtype(t, Value) || isSubtype(t, Value?) { return new t(this, obj); } else if t == NoneType { + // returning NoneType can be used to ignore a return value + // but if its not actually None, we still need to decrement the reference count + if obj != Py_None then Py_DECREF(obj); return None; } else { halt("Unsupported fromPython type: '" + t:string + "'"); @@ -578,14 +578,13 @@ module Python { } /* - Converts an array to a python list + Converts an array to a python list. Returns new reference */ @chpldoc.nodoc proc toList(arr): PyObjectPtr throws where isArrayType(arr.type) && arr.rank == 1 && arr.isDefaultRectangular() { var pyList = PyList_New(arr.size.safeCast(c_size_t)); this.checkException(); - this.toFree.pushBack(pyList); for i in 0.. 42, "key2" => 17]); + :arg retType: The Chapel type of the return value. If the callable returns nothing, use :type:`NoneType`. :arg args: The arguments to pass to the callable. + :arg kwargs: The keyword arguments to pass to the callable. */ + pragma "last resort" + proc this(type retType, + const args..., + kwargs:?=none): retType throws + where kwargs.isAssociative() { + var pyArg = this.packTuple((...args)); + defer Py_DECREF(pyArg); + return callInternal(retType, pyArg, kwargs); + } + pragma "last resort" + @chpldoc.nodoc + proc this(type retType, + kwargs:?=none): retType throws where kwargs.isAssociative() { + var pyArgs = Py_BuildValue("()"); + defer Py_DECREF(pyArgs); + return callInternal(retType, pyArgs, kwargs); + } + @chpldoc.nodoc proc this(type retType, const args...): retType throws { - var pyArgs: args.size * PyObjectPtr; - for param i in 0..#args.size { - pyArgs(i) = interpreter.toPython(args(i)); - } - - var pyRes; - if pyArgs.size == 1 then - pyRes = PyObject_CallOneArg(this.get(), pyArgs(0)); - else - pyRes = PyObject_CallFunctionObjArgs(this.get(), (...pyArgs), nil); - interpreter.checkException(); - interpreter.toFree.pushBack(pyRes); - - var res = interpreter.fromPython(retType, pyRes); - return res; + var pyArg = this.packTuple((...args)); + defer Py_DECREF(pyArg); + return callInternal(retType, pyArg, none); } - /* - Treat this value as a callable and call it with no arguments. - - See :proc:`~Value.this` for more information. - */ + @chpldoc.nodoc proc this(type retType): retType throws { var pyRes = PyObject_CallNoArgs(this.get()); - interpreter.checkException(); - interpreter.toFree.pushBack(pyRes); + this.check(); var res = interpreter.fromPython(retType, pyRes); return res; } - /* - Treat this value as a callable and call it with the given arguments and keyword arguments. - The keyword arguments should be passed as an associative Chapel array. - For example: - - .. code-block:: chapel - - var res = myObj(int, arg1, arg2, kwargs=["key1" => 42, "key2" => 17]); - - See :proc:`~Value.this` for more information. - */ - pragma "last resort" - proc this(type retType, const args..., kwargs:?t=none): retType throws - where kwargs.isAssociative() { + @chpldoc.nodoc + proc packTuple(const args...) throws { var pyArgs: args.size * PyObjectPtr; for param i in 0..#args.size { pyArgs(i) = interpreter.toPython(args(i)); } - var pyArg = PyTuple_Pack(args.size.safeCast(c_size_t), (...pyArgs)); + defer for pyArg in pyArgs do Py_DECREF(pyArg); - var pyKwargs; - if t != nothing { - pyKwargs = interpreter.toPython(kwargs); - } else { - pyKwargs = nil; - } - - var pyRes = PyObject_Call(this.get(), pyArg, pyKwargs); + var pyArg = PyTuple_Pack(args.size.safeCast(c_size_t), (...pyArgs)); interpreter.checkException(); - interpreter.toFree.pushBack(pyRes); - var res = interpreter.fromPython(retType, pyRes); - return res; + return pyArg; } - /* - Treat this value as a callable and call it with the given keyword arguments. - - The keyword arguments should be passed as an associative Chapel array. - For example: - - .. code-block:: chapel - - var res = myObj(int, kwargs=["key1" => 42, "key2" => 17]); - - See :proc:`~Value.this` for more information. - */ - pragma "last resort" - proc this(type retType, kwargs:?t=none): retType throws - where kwargs.isAssociative() { - - var pyArg = Py_BuildValue("()"); - + @chpldoc.nodoc + proc callInternal(type retType, + pyArg: PyObjectPtr, kwargs: ?t): retType throws { var pyKwargs; - if t != nothing { - pyKwargs = interpreter.toPython(kwargs); - } else { - pyKwargs = nil; - } + if t != nothing then pyKwargs = interpreter.toPython(kwargs); + else pyKwargs = nil; - var pyRes = PyObject_Call(this.get(), pyArg, pyKwargs); - interpreter.checkException(); - interpreter.toFree.pushBack(pyRes); + var pyRes = PyObject_Call(this.obj, pyArg, pyKwargs); + this.check(); var res = interpreter.fromPython(retType, pyRes); return res; @@ -1101,7 +1070,7 @@ module Python { Import a Python module by name. */ proc init(interpreter: borrowed Interpreter, in modName: string) { - super.init(interpreter, PyImport_ImportModule(modName.c_str())); + super.init(interpreter, PyImport_ImportModule(modName.c_str()), isOwned=true); this.modName = modName; } } @@ -1114,15 +1083,15 @@ module Python { var fnName: string; proc init(mod: borrowed Module, in fnName: string) { - super.init(mod.interpreter, PyObject_GetAttrString(mod.get(), fnName.c_str())); + super.init(mod.interpreter, PyObject_GetAttrString(mod.get(), fnName.c_str()), isOwned=true); this.fnName = fnName; } - proc init(interpreter: borrowed Interpreter, in fnName: string, in obj: PyObjectPtr) { - super.init(interpreter, obj); + proc init(interpreter: borrowed Interpreter, in fnName: string, in obj: PyObjectPtr, isOwned: bool = true) { + super.init(interpreter, obj, isOwned=isOwned); this.fnName = fnName; } proc init(interpreter: borrowed Interpreter, in lambdaFn: string) throws { - super.init(interpreter, nil:PyObjectPtr); + super.init(interpreter, nil:PyObjectPtr, isOwned=true); this.fnName = ""; init this; this.obj = interpreter.compileLambda(lambdaFn); @@ -1136,27 +1105,22 @@ module Python { class Class: Value { var className: string; proc init(mod: borrowed Module, in className: string) { - super.init(mod.interpreter, PyObject_GetAttrString(mod.get(), className.c_str())); + super.init(mod.interpreter, PyObject_GetAttrString(mod.get(), className.c_str()), isOwned=true); this.className = className; } @chpldoc.nodoc proc newInstance(const args...): PyObjectPtr throws { - var pyArgs: args.size * PyObjectPtr; - for param i in 0..#args.size { - pyArgs(i) = interpreter.toPython(args(i)); - } - - var pyRes = PyObject_CallFunctionObjArgs(this.get(), (...pyArgs), nil); - interpreter.checkException(); - + var pyArg = packTuple((...args)); + defer Py_DECREF(pyArg); + var pyRes = PyObject_Call(this.get(), pyArg, nil); + this.check(); return pyRes; } @chpldoc.nodoc proc newInstance(): PyObjectPtr throws { var pyRes = PyObject_CallNoArgs(this.get()); - interpreter.checkException(); - + this.check(); return pyRes; } @@ -1176,12 +1140,14 @@ module Python { */ class ClassObject: Value { @chpldoc.nodoc - proc init(interp: borrowed Interpreter, in obj: PyObjectPtr) { - super.init(interp, obj); + proc init(interp: borrowed Interpreter, in obj: PyObjectPtr, isOwned: bool = true) { + super.init(interp, obj, isOwned=isOwned); } proc setAttr(attr: string, value) throws { var pyValue = interpreter.toPython(value); + defer Py_DECREF(pyValue); + PyObject_SetAttrString(this.get(), attr.c_str(), pyValue); interpreter.checkException(); } @@ -1191,8 +1157,11 @@ module Python { for param i in 0..#args.size { pyArgs(i) = interpreter.toPython(args(i)); } + defer for pyArg in pyArgs do Py_DECREF(pyArg); var methodName = interpreter.toPython(method); + defer Py_DECREF(methodName); + var pyRes = PyObject_CallMethodObjArgs( this.get(), methodName, (...pyArgs), nil); interpreter.checkException(); @@ -1202,6 +1171,8 @@ module Python { } proc call(type retType, method: string): retType throws { var methodName = interpreter.toPython(method); + defer Py_DECREF(methodName); + var pyRes = PyObject_CallMethodNoArgs(this.get(), methodName); interpreter.checkException(); @@ -1427,6 +1398,7 @@ module Python { extern proc Py_INCREF(obj: PyObjectPtr); extern "chpl_Py_DECREF" proc Py_DECREF(obj: PyObjectPtr); extern "chpl_Py_CLEAR" proc Py_CLEAR(obj: c_ptr(PyObjectPtr)); + extern proc PyMem_Free(ptr: c_ptr(void)); extern proc PyObject_Str(obj: PyObjectPtr): PyObjectPtr; // `str(obj)` extern proc PyImport_ImportModule(name: c_ptrConst(c_char)): PyObjectPtr; diff --git a/test/library/packages/Python/examples/bs4/python_libs.notest b/test/library/packages/Python/examples/bs4/python_libs.notest new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/library/packages/Python/examples/numba/python_libs.notest b/test/library/packages/Python/examples/numba/python_libs.notest new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/library/packages/Python/examples/torch/myModel.chpl b/test/library/packages/Python/examples/torch/myModel.chpl index e3574b24a2e0..6f7f020618f0 100644 --- a/test/library/packages/Python/examples/torch/myModel.chpl +++ b/test/library/packages/Python/examples/torch/myModel.chpl @@ -17,7 +17,8 @@ proc main() { // create the model var model = myModelClass(); - var input_tensor = tensor(owned Value, [[1.0,]], kwargs=["requires_grad" => true]); + var input_tensor = tensor(owned Value, [[1.0,]], kwargs=["requires_grad" => false]); + writeln("Input tensor: ", input_tensor); // init model weights to 0.9 { @@ -34,6 +35,7 @@ proc main() { // compute the loss var loss_fn = MSELoss(); var target = tensor(owned Value, [[2.0,]]); + writeln("Target: ", target); var loss = loss_fn(owned ClassObject, pred, target); loss.call(NoneType, "backward"); diff --git a/test/library/packages/Python/examples/torch/mymodel.py b/test/library/packages/Python/examples/torch/mymodel.py index 6066896732b4..ff214df8e273 100644 --- a/test/library/packages/Python/examples/torch/mymodel.py +++ b/test/library/packages/Python/examples/torch/mymodel.py @@ -11,7 +11,8 @@ def forward(self, x): def main(): # create the model model = MyModel() - input_tensor = torch.tensor([[1.0]], requires_grad=True) + input_tensor = torch.tensor([[1.0]], requires_grad=False) + print("Input tensor:", input_tensor) # init model weights to 0.9 for p in model.parameters(): @@ -23,6 +24,7 @@ def main(): # compute the loss loss_fn = torch.nn.MSELoss() target = torch.tensor([[2.0]]) + print("Target:", target) loss = loss_fn(pred, target) loss.backward() diff --git a/test/library/packages/Python/examples/torch/python_libs.notest b/test/library/packages/Python/examples/torch/python_libs.notest new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/library/packages/Python/memleaks/lambdas.chpl b/test/library/packages/Python/memleaks/lambdas.chpl index 2920ea97c718..5388ea8560c7 100644 --- a/test/library/packages/Python/memleaks/lambdas.chpl +++ b/test/library/packages/Python/memleaks/lambdas.chpl @@ -8,6 +8,18 @@ proc main() { writeln(l(list(int), 1)); writeln(l(list(string), "hi")); - // this has a an extra lis + // this has an extra list writeln(l(list(list(int)), [1, 2, 3])); + + // chapelList([4,5,6]) + var l2 = new Function(interp, "lambda x,y,z,: [x,y,z]"); + // printing this out will cause a memleak, the __str__ method for list leaks + // e.g., 'print([[1,2,3])' in python leaks memory + l2(list(owned Value?), 1, "hi", [4, 5, 6]); + writeln("skipping potentially leaking print"); + + + var l3 = new Function(interp, "lambda x,: x"); + writeln(l3(list(int), [1,])); + writeln(l3(NoneType, [1,])); } diff --git a/test/library/packages/Python/memleaks/lambdas.good b/test/library/packages/Python/memleaks/lambdas.good index cea54b84f82e..af3cdb4bfafe 100644 --- a/test/library/packages/Python/memleaks/lambdas.good +++ b/test/library/packages/Python/memleaks/lambdas.good @@ -1,3 +1,6 @@ [1, 1] [hi, hi] [[1, 2, 3], [1, 2, 3]] +skipping potentially leaking print +[1] +()