diff --git a/src/lvpy_runtime.c b/src/lvpy_runtime.c index 88f70f5..a025237 100644 --- a/src/lvpy_runtime.c +++ b/src/lvpy_runtime.c @@ -18,6 +18,7 @@ static int lvpy_lock_save_key = -1; static void lvpy_drain_pending_releases_if_idle(void); static void lvpy_unlock_nodrain(void); +static void lvpy_defer_release(PyObject *obj); typedef struct { PyTypeObject *type; @@ -283,7 +284,9 @@ static PyObject *py_lv_struct_cast(PyObject *cls, PyObject *args) return NULL; } void *ptr = mp_to_ptr(ptr_obj); + lvpy_drain_pending_releases_if_idle(); if (!ptr) { + if (PyErr_Occurred()) return NULL; Py_RETURN_NONE; } py_lv_struct_t *out = PyObject_New(py_lv_struct_t, (PyTypeObject *)type_obj); @@ -302,8 +305,18 @@ static PyObject *py_lv_cast_instance(PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "O", &ptr_obj)) { return NULL; } - ((py_lv_struct_t *)self)->data = mp_to_ptr(ptr_obj); - return self; + void *ptr = mp_to_ptr(ptr_obj); + lvpy_drain_pending_releases_if_idle(); + if (!ptr && PyErr_Occurred()) return NULL; + py_lv_struct_t *s = (py_lv_struct_t *)self; + if (s->data && s->owns_data && s->data != ptr) { + /* The struct now views someone else's memory: let go of its own. */ + lvpy_release_struct_holder(s->data); + free(s->data); + } + if (s->data != ptr) s->owns_data = 0; + s->data = ptr; + return Py_NewRef(self); } static PyMethodDef py_base_struct_methods[] = { @@ -705,12 +718,22 @@ void *mp_to_ptr(PyObject *obj) return ((py_lv_struct_t *)obj)->data; } if (PyObject_CheckBuffer(obj)) { - Py_buffer view; - if (PyObject_GetBuffer(obj, &view, PyBUF_SIMPLE) == 0) { - void *p = view.buf; - PyBuffer_Release(&view); - return p; + /* The pointer is only valid while the buffer is exported. Hold the + * export in a memoryview until the outermost LVGL call returns, so + * Python code run during the call (an event callback, say) can't + * resize or free the memory LVGL is reading. */ + PyObject *view = PyMemoryView_FromObject(obj); + if (!view) return NULL; + Py_buffer *b = PyMemoryView_GET_BUFFER(view); + if (!PyBuffer_IsContiguous(b, 'C')) { + Py_DECREF(view); + PyErr_Format(PyExc_BufferError, "'%s' buffer is not contiguous", + Py_TYPE(obj)->tp_name); + return NULL; } + void *p = b->buf; + lvpy_defer_release(view); + return p; } if (PyUnicode_Check(obj)) { return (void *)PyUnicode_AsUTF8(obj); @@ -1102,7 +1125,11 @@ PyObject *mp_lv_funcptr(PyObject *wrapper, void *lv_fun, void *lv_callback, if (!lv_fun) Py_RETURN_NONE; if (lv_fun == lv_callback) { PyObject *callbacks = get_callback_dict_from_user_data(user_data); - if (callbacks) return PyDict_GetItemString(callbacks, func_name); + if (callbacks) { + /* The dict owns its value: return a reference of our own. */ + PyObject *cb = PyDict_GetItemString(callbacks, func_name); + if (cb) return Py_NewRef(cb); + } } Py_RETURN_NONE; } diff --git a/tests/test_runtime_references.py b/tests/test_runtime_references.py new file mode 100644 index 0000000..02cf056 --- /dev/null +++ b/tests/test_runtime_references.py @@ -0,0 +1,138 @@ +# SPDX-License-Identifier: MIT +"""Reference counts and buffer exports the runtime hands across to LVGL. + +- Reading a callback field back from a struct (``drv.open_cb``) returns a + new reference to the Python callback; reading it leaves the + callback's reference count where it was. ``__cast_instance__`` likewise + returns its struct with a reference of its own, and a struct that owned + its memory lets go of it rather than freeing it twice. +- A buffer passed where LVGL takes a ``void *`` (``obj.send_event(code, + buf)``) stays exported until the LVGL call returns, so Python code run + during the call can't resize it out from under the pointer LVGL holds. + The export ends when the call returns. + +The checks run in one subprocess under MALLOC_PERTURB_ so a use-after-free +shows up as a crash rather than luck. +""" + +import json +import os +import subprocess +import sys +import unittest + + +_SCRIPT = r""" +import json +import sys + +import lvgl as lv + +lv.init() +disp = lv.display_create(64, 64) +disp.set_color_format(lv.COLOR_FORMAT.RGB565) +buf = lv.draw_buf_create(64, 64, lv.COLOR_FORMAT.RGB565, 0) +disp.set_draw_buffers(buf, None) +disp.set_render_mode(lv.DISPLAY_RENDER_MODE.PARTIAL) +scr = lv.screen_active() +out = {} + + +# A callback read back from a struct field is a new reference. +def open_cb(drv, path, mode): + return None + +drv = lv.fs_drv_t() +drv.init() +drv.open_cb = open_cb +keep = [open_cb] * 8 # spare references, so a miscount shows as a number, not a crash +before = sys.getrefcount(open_cb) +same = True +for _ in range(4): + if drv.open_cb is not open_cb: + same = False +out["funcptr_same_object"] = same +out["funcptr_refcount_delta"] = sys.getrefcount(open_cb) - before + + +# __cast_instance__ returns the struct with a reference of its own. +area = lv.area_t() +other = lv.area_t() +before = sys.getrefcount(area) +for _ in range(4): + area.__cast_instance__(other) +out["cast_instance_refcount_delta"] = sys.getrefcount(area) - before + + +# A buffer handed to LVGL stays exported for the whole call. +data = bytearray(16) +seen = {} + +def on_event(e): + try: + data.extend(bytes(1 << 16)) + seen["resized"] = True + except BufferError: + seen["resized"] = False + +code = lv.event_register_id() +btn = lv.button(scr) +btn.add_event_cb(on_event, code, None) +btn.send_event(code, data) +out["resized_during_call"] = seen.get("resized") +try: + data.extend(b"x") + out["resizable_after_call"] = True +except BufferError: + out["resizable_after_call"] = False + +btn.delete() +lv.deinit() +print("RESULT " + json.dumps(out)) +""" + + +def _run(): + env = dict(os.environ) + env["MALLOC_PERTURB_"] = "165" + proc = subprocess.run( + [sys.executable, "-c", _SCRIPT], + capture_output=True, + text=True, + timeout=120, + env=env, + ) + for line in proc.stdout.splitlines(): + if line.startswith("RESULT "): + return proc, json.loads(line[len("RESULT "):]) + return proc, None + + +class RuntimeReferenceTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.proc, cls.out = _run() + + def setUp(self): + if self.out is None: + self.fail( + "script did not finish (exit %s)\nstdout:\n%s\nstderr:\n%s" + % (self.proc.returncode, self.proc.stdout, self.proc.stderr) + ) + + def test_callback_field_read_is_a_new_reference(self): + self.assertTrue(self.out["funcptr_same_object"]) + self.assertEqual(self.out["funcptr_refcount_delta"], 0) + + def test_cast_instance_returns_a_new_reference(self): + self.assertEqual(self.out["cast_instance_refcount_delta"], 0) + + def test_buffer_stays_exported_during_the_call(self): + self.assertIs(self.out["resized_during_call"], False) + + def test_buffer_export_ends_with_the_call(self): + self.assertIs(self.out["resizable_after_call"], True) + + +if __name__ == "__main__": + unittest.main()