Fix three VM memory leaks: computed-property double creation, missing frees in gravity_core_free, orphaned GC gray-list buffer - #445
Merged
Conversation
…core_init
VALUE_FROM_OBJECT() evaluates its argument twice when
GRAVITY_USE_HIDDEN_INITIALIZERS is not set:
#define VALUE_FROM_OBJECT(obj) ((gravity_value_t){.isa = ((gravity_object_t *)(obj)->isa), .p = (gravity_object_t *)(obj)})
Six bind sites in gravity_core_init called computed_property_create()
inline inside that macro (Object.class, Object.meta, Int.min/max meta,
Float.min/max meta), so each of those computed properties was created
twice: one copy bound, the duplicate orphaned (~2.2KB leaked per
gravity_core_init).
Use the same temp-variable pattern the rest of gravity_core_init already
uses for every other computed property.
Also free the nine computed properties missing from gravity_core_free's
manual free list (Object.class, Object.meta, Range.from, Range.to,
String.bytes, Int meta min/max, Float meta min/max) — previously leaked
on every core init/free cycle.
gravity_vm_new calls gravity_gc_setenabled(vm, true) before marray_init(vm->graylist). Enabling the GC can trigger a collection (gravity_gc_check -> gravity_gc_start), which grows the graylist buffer via marray_push/realloc. The marray_init that follows then zeroes the array struct, orphaning that buffer (leaked once per VM). Initialize graylist/gctemp before enabling the GC.
|
@barchett is attempting to deploy a commit to the sqlitecloud Team on Vercel. A member of the Team first needs to authorize it. |
Owner
|
Thanks @barchett |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Found while running an embedding host (repeated compile/run/free cycles) under AddressSanitizer/LeakSanitizer. Three distinct leaks, all reproducible with a minimal embed loop (below):
1. Computed properties created twice in
gravity_core_init(double macro expansion)When
GRAVITY_USE_HIDDEN_INITIALIZERSis not set,VALUE_FROM_OBJECT()evaluates its argument twice:Six bind sites in
gravity_core_initcallcomputed_property_create()inline inside that macro (Object.class,Object.meta,Intmetamin/max,Floatmetamin/max). Each of those computed properties is therefore created twice — one copy gets bound, the duplicate is orphaned immediately (~2.2KB leaked pergravity_core_init, even without ever freeing the core). The fix uses the same temp-variable pattern the rest ofgravity_core_initalready uses for every other computed property, plus a comment warning against callingcomputed_property_createinline inside the macro.2. Nine computed properties missing from
gravity_core_free's manual free listgravity_core_freemanually freesList.count,Map.count,Range.count,String.length,Int/Float.radians/degrees, andSystem.gcEnabled— but notObject.class,Object.meta,Range.from,Range.to,String.bytes,Intmetamin/max, orFloatmetamin/max, whichgravity_core_initalso creates. Each core init/free cycle leaked these (~3.3KB per cycle for hosts that restart the script engine).3. GC gray-list buffer orphaned in
gravity_vm_newgravity_vm_newcallsgravity_gc_setenabled(vm, true)beforemarray_init(vm->graylist). Enabling the GC can trigger a collection (gravity_gc_check→gravity_gc_start), which grows the gray-list buffer viamarray_push/realloc— and themarray_initthat follows zeroes the array struct, orphaning that buffer (one buffer leaked per VM). Fixed by initializinggraylist/gctempbefore enabling the GC.Repro
Built with
-fsanitize=address: before —SUMMARY: AddressSanitizer: 22296 byte(s) leaked in 156 allocation(s); after — clean exit, zero leaks.Testing
make && test/unittest/run_all.sh: 350/350 pass, 0 failed, 0 timed out with the patches applied.🤖 Generated with Claude Code