Fixes relating to memory management #35
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. Pointer Safety and Type Alignment
The most critical fix addressed a direct memory corruption bug in the cursor fetching logic where a Python object was passed to a function expecting a C structure.
The Issue: Python Object vs. C Pointer Mismatch
In the original code for
usqlite_cursor_fetchoneandfetchmany, thestepExecutefunction was called with the rawmp_obj_t. On the RP2350, dereferencing this resulted in an invalid memory access (Hard Fault).Original Code :
The Fix: Pointer Correction
The logic was updated to pass the extracted C structure pointer (
self) instead of the Python object wrapper.Fixed Code:
2. Transient Data Binding
I modified how Python strings and bytes are bound to SQL parameters to prevent "Use-After-Free" crashes caused by the MicroPython Garbage Collector (GC).
The Issue: Garbage Collection Race Conditions
The original code used
NULLas the destructor for SQLite bindings, assuming the memory was static. However, MicroPython's GC can move or delete these objects during execution.Original Code:
The Fix: SQLITE_TRANSIENT
((void*)-1)The destructor was changed to
(void*)-1. This forces SQLite to make a private copy of the data immediately, insulating it from GC movements.Fixed Code:
3. Cursor Lifecycle and Memory Leaks
Resolved a major memory leak where cursors were never removed from the connection's internal tracking list.
The Issue: Zombie Cursor Tracking
The original
usqlite_cursor_closefunction finalized the SQL statement but left the cursor entry in the connection's list, causing Out of Memory (OOM) errors over time.The Fix: Deregistration Logic
Added a call to
usqlite_connection_deregisterto ensure the cursor is fully unlinked from the connection.Fixed Code:
4. Connection Cleanup Synchronization
Overhauled the database closing sequence to prevent crashes during shutdown.
The Issue: Unsafe List Iteration
The original closing logic attempted to iterate through the cursor list using standard index increments. This is unsafe because closing a cursor now removes it from the list, shifting all subsequent indices.
The Fix: Safe While-Loop Iteration
The loop now always targets the first item (
items[0]) while the list length is greater than zero.Fixed Code:
Detailed Analysis: Connection Memory Management
This section specifically addresses flaws found in
usqlite_connection.c.The Problem
Array Index Bug: The loop intended to clear every cursor but hard-coded
items[0]while iterating withi, leaving dangling pointers for all items except the first.Illegal GC Memory Free: The code used
m_freeon objects managed by the MicroPython GC. This corrupts heap metadata, leading to crashes during the next GC cycle.Fixed Implementation
The corrected version uses the proper index, removes illegal
m_freecalls, and resets the list state properly.All these changed have been performed with the thought of making it work on a raspberry pi pico 2W. Since the program was constantly freezing before this.