fuzzball-muck / fuzzball

Ongoing development of the Fuzzball MUCK server software and associated functionality.
Other
46 stars 26 forks source link

memory leak from nested arrays and array_pin #199

Open charlesreiss opened 6 years ago

charlesreiss commented 6 years ago

Nested arrays and array_pin can leak memory. For example, the MUF program:

: main { }dict ARRAY_PIN DUP 0 ARRAY_SETITEM DUP DUP ARRAY_SETITEM ;

leaks memory (AddressSanitizer report):

=================================================================
==7216==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x4d0730 in calloc (/home/charles/software/fbmuck-code2/src/fbmuck+0x4d0730)
    #1 0x50ca16 in array_tree_alloc_node /home/charles/software/fbmuck-code2/src/array.c:303:31
    #2 0x50ca16 in array_tree_insert /home/charles/software/fbmuck-code2/src/array.c:355
    #3 0x50a9e4 in array_setitem /home/charles/software/fbmuck-code2/src/array.c:931:7
    #4 0x5d287c in prim_array_setitem /home/charles/software/fbmuck-code2/src/p_array.c:338:14
    #5 0x57b4c2 in interp_loop /home/charles/software/fbmuck-code2/src/interp.c:1726:3
    #6 0x5bd24a in do_move /home/charles/software/fbmuck-code2/src/move.c:485:6
    #7 0x568b68 in process_command /home/charles/software/fbmuck-code2/src/game.c
    #8 0x6e3d17 in do_command /home/charles/software/fbmuck-code2/src/interface.c:1000:6
    #9 0x6e3d17 in process_commands /home/charles/software/fbmuck-code2/src/interface.c:1071
    #10 0x6e3d17 in shovechars /home/charles/software/fbmuck-code2/src/interface.c:2324
    #11 0x6e1991 in main /home/charles/software/fbmuck-code2/src/interface.c:4532:2
    #12 0x7f8d7e11b3f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x4d0538 in malloc (/home/charles/software/fbmuck-code2/src/fbmuck+0x4d0538)
    #1 0x509971 in new_array /home/charles/software/fbmuck-code2/src/array.c:526:24
    #2 0x509971 in new_array_dictionary /home/charles/software/fbmuck-code2/src/array.c:574
    #3 0x5ceff9 in prim_array_make_dict /home/charles/software/fbmuck-code2/src/p_array.c:75:10
    #4 0x57b4c2 in interp_loop /home/charles/software/fbmuck-code2/src/interp.c:1726:3
    #5 0x5bd24a in do_move /home/charles/software/fbmuck-code2/src/move.c:485:6
    #6 0x568b68 in process_command /home/charles/software/fbmuck-code2/src/game.c
    #7 0x6e3d17 in do_command /home/charles/software/fbmuck-code2/src/interface.c:1000:6
    #8 0x6e3d17 in process_commands /home/charles/software/fbmuck-code2/src/interface.c:1071                              
    #9 0x6e3d17 in shovechars /home/charles/software/fbmuck-code2/src/interface.c:2324                                        
    #10 0x6e1991 in main /home/charles/software/fbmuck-code2/src/interface.c:4532:2
    #11 0x7f8d7e11b3f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291                        

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 2 allocation(s).
wyld-sw commented 6 years ago

@woggle, do you have time to tackle this one?

charlesreiss commented 6 years ago

I'll start on it, but I'm not sure I'll have much time in the next couple weeks.

My proposal for fixing this:

After doing this, a second, less urgent stage to the fix would be:

tanabi commented 6 months ago

Initial investigation reveals that fork does indeed deep copy already and doesn't share. Looks like event_send also doesn't share and does a deep copy. That said, deep copy will need to support whatever tracking method we use.

tanabi commented 6 months ago

It looks like @charlesreiss did the bulk of the work here. What remains to be done is:

1) Verify fork and event_send are the only two MUFs that pass arrays between programs (easier said than done ...) and

2) add limits to the array memory allocations, as indeed, they can currently allocate an essentially infinite amount of memory. That said, they are technically limited by the instruction limits, so there is an upper limit to how destructive this can be. So I'm wondering if there's really anything to do here? I tend to feel like the instruction limits would limit how big the array can get, and even if you made a loop that does nothing but pack full sized buffers into an array, you'd probably run out of instructions before doing something destructive.

[2] is easily enough checked by making a potentially abusive MUF program and setting it M2, then seeing what carnage is wrought, so I'll do that on my local dev. :)