• Kirill Smelkov's avatar
    golang: pychan: Fix memory leak in .from_chan_* pychan <- chan[X] wrappers · 2ec5e96b
    Kirill Smelkov authored
    The code of pychan_from_raw, that pychan.from_chan_X calls, was creating
    pychan object and then setting pychan._ch to the specified raw channel.
    But it missed that pychan.__new__ was creating full Python object, with
    everything initialized - in particular with pychan._ch initialized to
    another channel created by pychan.__cinit__ constructor, and so pointer
    to that another channel was removed without decrefing it first. That
    caused the leak of that second channel observable as the following
    LeakSanitizer report when run on e.g. added test:
    
        Direct leak of 72 byte(s) in 1 object(s) allocated from:
            #0 0x7f70902f3bd7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
            #1 0x7f708bfab612 in zalloc golang/runtime/libgolang.cpp:1307
            #2 0x7f708bfa56c0 in _makechan golang/runtime/libgolang.cpp:469
            #3 0x7f708be78da2 in __pyx_f_6golang_7_golang__makechan_pyexc golang/_golang.cpp:8844
            #4 0x7f708be703ad in __pyx_pf_6golang_7_golang_6pychan___cinit__ golang/_golang.cpp:4870
            #5 0x7f708be7019d in __pyx_pw_6golang_7_golang_6pychan_1__cinit__ golang/_golang.cpp:4833
            #6 0x7f708beaa0f8 in __pyx_tp_new_6golang_7_golang_pychan golang/_golang.cpp:29896
            #7 0x7f708be743af in __pyx_f_6golang_7_golang_pychan_from_raw golang/_golang.cpp:7240
            #8 0x7f708be73e76 in __pyx_f_6golang_7_golang_6pychan_from_chan_int golang/_golang.cpp:6927
            #9 0x7f7088a990a5 in __pyx_pf_6golang_12_golang_test_62test_pychan_from_raw_noleak golang/_golang_test.cpp:7479
            #10 0x7f7088a98ef2 in __pyx_pw_6golang_12_golang_test_63test_pychan_from_raw_noleak golang/_golang_test.cpp:7445
    
    -> Fix it by adjusting raw chan -> py chan conversion routine to first
    create uninitialized py object - with no underlying channel created at all.
    
    Adjust pynil to use pychan_from_raw instead of duplicating its code, so
    that we keep the logic and the fix only in one place.
    
    The context where this problem was originally discovered is xamari from
    XLTE where on every request new timer is created to handle request
    timeout, and that timer, being Python-level object, wraps underlying
    C-level timer with creating pychan wrapper of that:
    
    https://lab.nexedi.com/kirr/xlte/-/blob/8e606c64/amari/__init__.py#L182-193
    https://lab.nexedi.com/nexedi/pygolang/-/blob/6dd420da/golang/_time.pyx#L96
    https://lab.nexedi.com/nexedi/pygolang/-/blob/6dd420da/golang/_time.pyx#L104
    
    As the result on every request memory was leaked on and on.
    
    Besides new test going ok even under LeakSanitizer, the following test
    program confirms the fix:
    
    ```py
    from golang import context
    
    def main():
        bg = context.background()
        key = object()
        while 1:
            ctx = context.with_value(bg, key, 1)
    
    main()
    ```
    
    Before the patch it leaks ~ 1GB of RAM every second on my computer due
    to similar raw chan -> py chan wrapping in py context.with_value
    
    https://lab.nexedi.com/nexedi/pygolang/-/blob/6dd420da/golang/_context.pyx#L169-180
    https://lab.nexedi.com/nexedi/pygolang/-/blob/6dd420da/golang/_context.pyx#L38-43
    
    After the fix that program stays at constant RSS usage forever.
    
    /cc ORS team (@jhuge, @lu.xu, @tomo, @xavier_thompson, @Daetalus)
    /reviewed-by @jerome
    /reviewed-on !24
    2ec5e96b