Skip to content

Fix memory leaks and TODOs in C source #29

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.

Sign up for GitHub

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

Merged
benoitc merged 3 commits into from
Mar 14, 2026
Merged

Fix memory leaks and TODOs in C source #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90fdf19
Fix PyDict_New memory leak in py_subinterp_thread.c
benoitc Mar 14, 2026
fa32305
Add logging for queue capacity warning in py_event_loop.c
benoitc Mar 14, 2026
f6a84a1
Use py_to_term for result serialization in py_subinterp_thread.c
benoitc Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion c_src/py_convert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ static inline bool is_numpy_ndarray(PyObject *obj) {
*
* @see term_to_py() for the reverse conversion
*/
static ERL_NIF_TERM py_to_term(ErlNifEnv *env, PyObject *obj) {
ERL_NIF_TERM py_to_term(ErlNifEnv *env, PyObject *obj) {
/*
* Type check ordering optimized for web/ASGI workloads:
* 1. Strings (most common in HTTP headers, bodies, JSON)
Expand Down
7 changes: 4 additions & 3 deletions c_src/py_event_loop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2774,9 +2774,10 @@ bool event_loop_add_pending(erlang_event_loop_t *loop, event_type_t type,
loop->pending_capacity = new_capacity;
/* Note: Linked list doesn't need realloc, just the capacity limit */
} else {
/* At hard cap - log warning but don't drop silently */
/* TODO: Add proper logging mechanism */
return false; /* Queue at maximum capacity */
/* At hard cap - warn and reject */
fprintf(stderr, "event_loop_add_pending: queue at maximum capacity (%zu), rejecting event\n",
(size_t)MAX_PENDING_CAPACITY);
return false;
}
}

Expand Down
2 changes: 1 addition & 1 deletion c_src/py_nif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ extern ERL_NIF_TERM ATOM_SPAN_EVENT; /**< @brief `span_event` atom */
* @note Does not consume a reference to obj
* @note May return ATOM_ERROR on allocation failure
*/
static ERL_NIF_TERM py_to_term(ErlNifEnv *env, PyObject *obj);
extern ERL_NIF_TERM py_to_term(ErlNifEnv *env, PyObject *obj);

/**
* @brief Convert an Erlang term to a Python object
Expand Down
76 changes: 24 additions & 52 deletions c_src/py_subinterp_thread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/

#include "py_subinterp_thread.h"
#include "py_nif.h"
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
Expand Down Expand Up @@ -429,6 +430,15 @@ static void *worker_thread_main(void *arg) {
PyObject *result = NULL;
PyObject *globals = ns ? ns->globals : PyDict_New();
PyObject *locals = ns ? ns->locals : PyDict_New();
bool owns_globals = (ns == NULL);
bool owns_locals = (ns == NULL);

/* Check allocation if we own the dicts */
if ((owns_globals && globals == NULL) || (owns_locals && locals == NULL)) {
if (owns_globals) Py_XDECREF(globals);
if (owns_locals) Py_XDECREF(locals);
break;
}

switch (header.req_type) {
case REQ_CALL:
Expand All @@ -447,6 +457,8 @@ static void *worker_thread_main(void *arg) {
} else if (enif_get_atom(tmp_env, elements[0], mod_str, 256, ERL_NIF_LATIN1)) {
/* Already filled */
} else {
if (owns_globals) Py_DECREF(globals);
if (owns_locals) Py_DECREF(locals);
break;
}

Expand All @@ -458,6 +470,8 @@ static void *worker_thread_main(void *arg) {
} else if (enif_get_atom(tmp_env, elements[1], func_str, 256, ERL_NIF_LATIN1)) {
/* Already filled */
} else {
if (owns_globals) Py_DECREF(globals);
if (owns_locals) Py_DECREF(locals);
break;
}

Expand All @@ -481,6 +495,8 @@ static void *worker_thread_main(void *arg) {

if (module == NULL) {
PyErr_Clear();
if (owns_globals) Py_DECREF(globals);
if (owns_locals) Py_DECREF(locals);
break;
}

Expand All @@ -490,6 +506,8 @@ static void *worker_thread_main(void *arg) {

if (func == NULL) {
PyErr_Clear();
if (owns_globals) Py_DECREF(globals);
if (owns_locals) Py_DECREF(locals);
break;
}

Expand Down Expand Up @@ -592,59 +610,13 @@ static void *worker_thread_main(void *arg) {
break;
}

/* Serialize result */
/* Clean up owned dicts after switch completes */
if (owns_globals) Py_DECREF(globals);
if (owns_locals) Py_DECREF(locals);

/* Serialize result using py_to_term for full type support */
if (success && result != NULL) {
/* For now, just return ok atom */
/* TODO: Proper py_to_term conversion and ETF serialization */
ERL_NIF_TERM result_term;
if (result == Py_None) {
result_term = enif_make_atom(tmp_env, "none");
} else if (PyLong_Check(result)) {
long val = PyLong_AsLong(result);
result_term = enif_make_long(tmp_env, val);
} else if (PyFloat_Check(result)) {
double val = PyFloat_AsDouble(result);
result_term = enif_make_double(tmp_env, val);
} else if (PyUnicode_Check(result)) {
Py_ssize_t size;
const char *str = PyUnicode_AsUTF8AndSize(result, &size);
if (str) {
ErlNifBinary bin;
if (enif_alloc_binary(size, &bin)) {
memcpy(bin.data, str, size);
result_term = enif_make_binary(tmp_env, &bin);
} else {
result_term = enif_make_atom(tmp_env, "conversion_error");
}
} else {
result_term = enif_make_atom(tmp_env, "conversion_error");
}
} else if (PyBool_Check(result)) {
result_term = result == Py_True ?
enif_make_atom(tmp_env, "true") :
enif_make_atom(tmp_env, "false");
} else {
/* Fallback: convert to string representation */
PyObject *str = PyObject_Str(result);
if (str) {
Py_ssize_t size;
const char *s = PyUnicode_AsUTF8AndSize(str, &size);
if (s) {
ErlNifBinary bin;
if (enif_alloc_binary(size, &bin)) {
memcpy(bin.data, s, size);
result_term = enif_make_binary(tmp_env, &bin);
} else {
result_term = enif_make_atom(tmp_env, "conversion_error");
}
} else {
result_term = enif_make_atom(tmp_env, "conversion_error");
}
Py_DECREF(str);
} else {
result_term = enif_make_atom(tmp_env, "pyobject");
}
}
ERL_NIF_TERM result_term = py_to_term(tmp_env, result);
Py_XDECREF(result);

/* Wrap in {ok, Result} */
Expand Down
Loading