From 294fbb2ad72648bbae204ce9c62ce5d2b1c9e12d Mon Sep 17 00:00:00 2001 From: Jacky Kwok <77720778+jackykwok2024@users.noreply.github.com> Date: Mon, 10 Jul 2023 20:20:10 -0400 Subject: [PATCH] Memory leak in Python target fixed (#246) * memory leak fix * Update core/lf_token.c Co-authored-by: Edward A. Lee * Update python_port.c It was incorporated to assist with debugging. * Update python/lib/python_port.c Co-authored-by: Edward A. Lee * Update python/lib/python_port.c Co-authored-by: Edward A. Lee * Update core/lf_token.c Co-authored-by: Marten Lohstroh * Update core/lf_token.c Co-authored-by: Marten Lohstroh * Update lf_token.c * Update python_port.c * Added back Py_INCREF(val) and define a new destructor for output port * Add python count decrement to the header file The function is called by the deserializer during federated execution. * Fixed SegFault during unfederated execution This commit fixes segfault for unfederated execution, but leads to memory leak during federated execution. * Minor cleanup. --------- Co-authored-by: Edward A. Lee Co-authored-by: Marten Lohstroh Co-authored-by: Peter Donovan --- core/lf_token.c | 17 +++++++++-------- python/include/python_port.h | 1 + python/lib/python_port.c | 28 +++++++++++++++++++++------- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/core/lf_token.c b/core/lf_token.c index 485fe183a..b309f0a2f 100644 --- a/core/lf_token.c +++ b/core/lf_token.c @@ -130,23 +130,24 @@ lf_token_t* lf_writable_copy(lf_port_base_t* port) { //////////////////////////////////////////////////////////////////// //// Internal functions. - void _lf_free_token_value(lf_token_t* token) { if (token->value != NULL) { // Count frees to issue a warning if this is never freed. _lf_count_payload_allocations--; // Free the value field (the payload). - // First check whether the value field is garbage collected (e.g. in the - // Python target), in which case the payload should not be freed. -#ifndef _LF_GARBAGE_COLLECTED LF_PRINT_DEBUG("_lf_free_token_value: Freeing allocated memory for payload (token value): %p", - token->value); - if (token->type->destructor == NULL) { - free(token->value); - } else { + token->value); + // First check the token's destructor field and invoke it if it is not NULL. + if (token->type->destructor != NULL) { token->type->destructor(token->value); } + // If Python Target is not enabled and destructor is NULL + // Token values should be freed + else { +#ifndef _PYTHON_TARGET_ENABLED + free(token->value); #endif + } token->value = NULL; } } diff --git a/python/include/python_port.h b/python/include/python_port.h index 5d36cb2c3..269abe756 100644 --- a/python/include/python_port.h +++ b/python/include/python_port.h @@ -98,4 +98,5 @@ typedef struct { FEDERATED_CAPSULE_EXTENSION } generic_port_capsule_struct; +void python_count_decrement(void* py_object); #endif diff --git a/python/lib/python_port.c b/python/lib/python_port.c index 005034c25..118281de3 100644 --- a/python/lib/python_port.c +++ b/python/lib/python_port.c @@ -36,9 +36,21 @@ THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "python_port.h" #include "reactor.h" +#include "api/api.h" +#include "api/set.h" PyTypeObject py_port_capsule_t; +//////////// destructor Function(s) ///////////// +/** + * Decrease the reference count of PyObject. When the reference count hits zero, + * Python can free its memory. + * @param py_object A PyObject with count 1 or greater. + */ +void python_count_decrement(void* py_object) { + Py_XDECREF((PyObject*)py_object); +} + //////////// set Function(s) ///////////// /** * Set the value and is_present field of self which is of type @@ -68,7 +80,7 @@ PyTypeObject py_port_capsule_t; * @param args contains: * - val: The value to insert into the port struct. */ -PyObject* py_port_set(PyObject *self, PyObject *args) { +PyObject* py_port_set(PyObject* self, PyObject* args) { generic_port_capsule_struct* p = (generic_port_capsule_struct*)self; PyObject* val = NULL; @@ -85,13 +97,15 @@ PyObject* py_port_set(PyObject *self, PyObject *args) { } if (val) { - LF_PRINT_DEBUG("Setting value %p.", val); - Py_XDECREF(port->value); - Py_INCREF(val); - // Call the core lib API to set the port - _LF_SET(port, val); - + LF_PRINT_DEBUG("Setting value %p with reference count %d.", val, (int) Py_REFCNT(val)); + //Py_INCREF(val); + //python_count_decrement(port->value); + + lf_token_t* token = lf_new_token((void*)port, val, 1); + lf_set_destructor(port, python_count_decrement); + lf_set_token(port, token); Py_INCREF(val); + // Also set the values for the port capsule. p->value = val; p->is_present = true;