pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.49k stars 524 forks source link

AsyncMachine: Memory leak happend #475

Closed h-nakai closed 3 years ago

h-nakai commented 3 years ago

Version: 0.8.3 (also occurred in 0.8.2)

We've been using Machine, but we've adopted AsyncMachine to add a new API call process. However, the use of asyncmachine caused a memory leak.

I also tried gc.collect(), but it didn't improve it. (return value is 0 and there are no content in gc.garbage)

small sample code is included here.

import asyncio
from transitions.extensions.asyncio import AsyncMachine

class Model(object):
    def __init__(self):
        pass

async def async_machine() -> None:
    counter = 0
    states = ["start", "end"]
    transitions = [{
        "trigger": "next",
        "source": "start",
        "dest": "end"
    }]
    while True:
        model = Model()
        machine = AsyncMachine(model=model, states=states, initial="start", transitions=transitions, auto_transitions=False)
        await model.trigger("next")

asyncio.get_event_loop().run_until_complete(async_machine())
import asyncio
from transitions.extensions.asyncio import AsyncMachine

class Model(object):
    def __init__(self):
        pass

async def async_machine() -> None:
    counter = 0
    states = ["start", "end"]
    transitions = [{
        "trigger": "next",
        "source": "start",
        "dest": "end"
    }]
    machine = AsyncMachine(states=states, initial="start", transitions=transitions, auto_transitions=False)
    while True:
        model = Model()
        machine.add_model(model)
        await model.trigger("next")
        machine.remove_model(model)

asyncio.get_event_loop().run_until_complete(async_machine())
h-nakai commented 3 years ago

I found. The dict named async_task, size of this dictionary is on the rise

https://github.com/pytransitions/transitions/blob/dd787e1ba88408e2efb523bb7bd5c3fd423b9cdb/transitions/extensions/asyncio.py#L305

https://github.com/pytransitions/transitions/blob/dd787e1ba88408e2efb523bb7bd5c3fd423b9cdb/transitions/extensions/asyncio.py#L366

aleneum commented 3 years ago

Hello @h-nakai,

thanks for that very precice issue description. I never imagined that this could be a problem but if your use case actually involves creating that many models, I will implement a clean up for that dictionary.

h-nakai commented 3 years ago

@aleneum Thanks for the reply. We plan to create many sessions, so it would be helpful if you could make some corrections. I appreciate your help.

Tentatively. We'll going to override the Machine's init. In the init of the AsyncMachine, I'm going to define an async_task.

aleneum commented 3 years ago

Please checkout the branch dev-async-tasks and let me know if that tackles your issue. I added a test which creates 100 models and checks whether AsyncMachine.async_tasks is empty after all models had been used.

h-nakai commented 3 years ago

Thanks for the quick fix. I have confirmed that there are no memory leaks.

h-nakai commented 3 years ago

It has been running for a while, but so far it is working normally. We will be using this branch as a solidified whl file until this fix is released to pip.

Would you like me to close it once?

aleneum commented 3 years ago

Hi @h-nakai,

I released 0.8.4 which includes the fix for the memory leak. Please close this issue when you transitioned to 0.8.4. I will then delete the dev-async-tasks branch.

h-nakai commented 3 years ago

I got it.

We will check the operation within a few days. After that, we will close this ticket.

h-nakai commented 3 years ago

Correction confirmed. Close this ticket. thank you for quick fix, aleneum.