prompt-toolkit / python-prompt-toolkit

Library for building powerful interactive command line applications in Python
https://python-prompt-toolkit.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
9.11k stars 717 forks source link

change filters's order to fix memory leak #1746

Closed vicalloy closed 1 year ago

vicalloy commented 1 year ago

Reference: https://github.com/prompt-toolkit/python-prompt-toolkit/issues/1688

I updated to the latest python-prompt-toolkit and found that it still has memory leaks. If a filter cached by a global filter function, such as def is_done(), the filter cannot be freed. Filter operations should be very careful to avoid using global filters as the first filter.

Unresolved bug: If I called await session.prompt_async() one PromptSession is not freed.

import sys
import asyncio
import gc
from prompt_toolkit import PromptSession

async def create_session():
    try:
        session = PromptSession()
        # await session.prompt_async()
    except KeyboardInterrupt:
        print("pass")
    del session
    gc.collect()

async def m():
    for _ in range(5):
        await create_session()
    gc.collect()
    obj = gc.get_objects()
    print('len', len([o for o in obj if isinstance(o, PromptSession)]))

asyncio.run(m())
jonathanslenders commented 1 year ago

Thanks! The fix makes sense to me. I'm merging it, because it does fix the issue, and it's quite trivial. But long term, I have to think of a better solution.

So, the problem is that all filters have a cache within them, and if we do global_filter & filter_that_references_our_prompt_session then the global filter will hold a reference to our filter in the cache, which will hold a reference to the prompt session.

The cache is really important for performance, so it's quite tricky. I'm not sure what alternative we have.