Skip to content

Non-root component event handlers cause memory leaks #510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rmorshea opened this issue Sep 5, 2021 · 1 comment
Closed

Non-root component event handlers cause memory leaks #510

rmorshea opened this issue Sep 5, 2021 · 1 comment

Comments

@rmorshea
Copy link
Collaborator

rmorshea commented Sep 5, 2021

Originally posted by @mx781 in #509

The following code produces a memory leak because the layout does not release references to the onClick event handler. To observe the leak, just run top -p <PID> where the PID is printed from os.getpid(). We can confirm that this leak results from event handlers because when we remove it, the leak goes away:

import idom
import os


print(os.getpid())


@idom.component
def SomeComponent():
    flag, set_flag = idom.hooks.use_state(True)
    set_flag(not flag)
    return idom.html.div(
        idom.html.button({"onClick": set_flag}, flag),
    )


idom.run(SomeComponent, port=8000)

Interestingly though, if we assign the event handler at the root of the component's structure, the leak goes away:

import idom
import os


print(os.getpid())


@idom.component
def SomeComponent():
    flag, set_flag = idom.hooks.use_state(True)
    set_flag(not flag)
    return idom.html.button({"onClick": set_flag}, flag)


idom.run(SomeComponent, port=8000)

This shows that we're not cleaning up state resulting from elements not at the root of the component.

The leak appears to be pretty small though since I don't think we've every seen the docs crash as a result of running out of memory.

@rmorshea rmorshea added the bug label Sep 5, 2021
@rmorshea
Copy link
Collaborator Author

rmorshea commented Sep 5, 2021

closed by: db327b7

(accidentally committed to main instead of a branch for PR).

@rmorshea rmorshea closed this as completed Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant