sciter-sdk / pysciter

Python bindings for Sciter
https://sciter.com
MIT License
396 stars 40 forks source link

Memory leak w/ `sciter.Value.pack_args` #72

Closed AlecHaring closed 2 years ago

AlecHaring commented 2 years ago

The data that's passed to sciter.Value.pack_args, and then passed to _api.ValueCopy is seemingly never cleaned up.

Here's a demo that calls sciter.Window.call_function in an infinite loop with fake data:

import random
import string
import threading

import sciter

def gen_random_str(str_len: int) -> str:
    """
    Generates a random string of length str_len
    """
    return ''.join(random.choice(string.ascii_letters) for _ in range(str_len))

def gen_fake_data(n_keys: int) -> dict:
    """
    Generates a dict of n_keys keys with random values
    """
    data = {}
    for i in range(n_keys):
        data[gen_random_str(255)] = gen_random_str(255)
    return data

def background_task(frame: sciter.Window):
    """
    Memory leak demo
    """
    while True:
        frame.call_function('fake_func', gen_fake_data(500))

def main():
    frame = sciter.Window(ismain=True, uni_theme=True)
    threading.Thread(target=background_task, args=(frame,)).start()
    frame.minimal_menu()
    frame.load_file("test.htm")
    frame.run_app()

if __name__ == '__main__':
    main()

test.htm:

<html>
<head>
    <title>Test</title>
</head>
<body>
    <h1>Test</h1>
</body>
</html>

The sciter.value object that wraps the data is garbage collected, resulting in _api.ValueClear being called, but that doesn't seem to actually clear it from memory. Running the above demo for 5 minutes consumed 500 MB of memory

AlecHaring commented 2 years ago

I've simplified the demo:

import sciter
from sciter.capi.scvalue import SCITER_VALUE

def memory_leak():
    """
    Memory leak demo
    """
    while True:
        large_list = [0] * 100_000
        sciter_array_struct = SCITER_VALUE * 1
        sciter_array = sciter_array_struct()
        sv = sciter.Value(large_list)
        sv.copy_to(sciter_array[0])
        sv.clear()

if __name__ == '__main__':
    memory_leak()
c-smile commented 2 years ago

Essentially each struct VALUE shall be initialized by ValueInit(&val) and freed by ValueClear(&val)

Seems like sciter_array[0] is not cleared.

AlecHaring commented 2 years ago

Ah, I see. That makes a lot of sense now that you point it out. I really appreciate the nudge in the right direction.

sv's value is copied into sciter_array[0]'s position in memory. sv is cleaned up properly, but, as you said, sciter_array is not.

I've modified the above demo to the following:

import ctypes

import sciter
from sciter.value import _api
from sciter.capi.scvalue import SCITER_VALUE

def memory_leak():
    """
    Memory leak demo
    """
    while True:
        large_list = [0] * 100_000
        sciter_array_struct = SCITER_VALUE * 1
        sciter_array = sciter_array_struct()
        sv = sciter.Value(large_list)
        sv.copy_to(sciter_array[0])
        sv.clear()
        # free sciter_array from memory
        ptr = ctypes.pointer(sciter_array[0])
        _api.ValueClear(ptr)

if __name__ == '__main__':
    memory_leak()

And the problem went away.

So, now that that is figured out, I'll revise Value.pack_args to make it properly clean up after itself and will make a PR.

pravic commented 2 years ago

Oh. Thanks for the investigation, it's a total loss.

pravic commented 2 years ago

Merged, will be in 0.6.9