Skip to content

Exceptions can use up large amounts of memory #43

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
furbrain opened this issue Jul 8, 2023 · 8 comments · Fixed by #44
Closed

Exceptions can use up large amounts of memory #43

furbrain opened this issue Jul 8, 2023 · 8 comments · Fixed by #44

Comments

@furbrain
Copy link
Contributor

furbrain commented Jul 8, 2023

If an exception occurs in a task which is not the "main task", the exception is stored in a global variable: _exc_context in core.py. This is then passed to call_exception_handler. The exception contains the full traceback at the point of error - which includes references to multiple objects at the time of the error. These objects cannot then be garbage collected until the next exception occurs and _exc_context is overwritten. In complex programs this can cause large amounts of memory to be used

@furbrain
Copy link
Contributor Author

furbrain commented Jul 8, 2023

Example code:

import gc
import asyncio

def show_mem(text: str):
    gc.collect()
    print(f"{text} mem: {gc.mem_alloc()}")


async def crashing_routine(obj):
    print(f"object is {len(obj)}")
    show_mem("Before crash")
    await asyncio.sleep(0.5)
    raise RuntimeError("Crash")

async def main():
    show_mem("Before large_object creation")
    big_object = bytearray(8000)
    show_mem("After large_object creation")
    asyncio.create_task(crashing_routine(big_object))
    await asyncio.sleep(2)

show_mem("Before running")
asyncio.run(main())
show_mem("after crash")
asyncio.run(main())
show_mem("after second run")

Output:

code.py output:
Before running mem: 3952
Before large_object creation mem: 4096
After large_object creation mem: 12112
object is 8000
Before crash mem: 12208
Traceback (most recent call last):
  File "asyncio/core.py", line 246, in run_until_complete
  File "code.py", line 13, in crashing_routine
RuntimeError: Crash
after crash mem: 12304
Before large_object creation mem: 12432
After large_object creation mem: 20448
object is 8000
Before crash mem: 20544
Traceback (most recent call last):
  File "asyncio/core.py", line 246, in run_until_complete
  File "code.py", line 13, in crashing_routine
RuntimeError: Crash
after second run mem: 12304

Code done running.

As you can see after crash mem still has 8k of memory assigned for big_object, even though it is no longer in scope.
If we try to re-run the code then memory usage goes up to 20k - we have two instances of big_object in memory which could cause memory issues

@furbrain
Copy link
Contributor Author

furbrain commented Jul 8, 2023

I think this could be fixed by simply generating a context dict on the fly before calling call_exception_handler - but is there a reason why there is a global variable there?

@dhalbert
Copy link
Contributor

dhalbert commented Jul 8, 2023

If you have the inclination to try your hypothesis, and it works, a PR would be welcome. Nearly all the code in this library is copied from the MicroPython asyncio implementation: https://github.com/micropython/micropython/tree/master/extmod/asyncio. It's worth checking the latest version of that to see if it's changed.

@furbrain
Copy link
Contributor Author

furbrain commented Jul 8, 2023

No new changes in the micropython version. I'll submit a PR shortly. Is it worth submitting the same issue in the micropython repo?

@dhalbert
Copy link
Contributor

dhalbert commented Jul 8, 2023

I don't know if their traceback handling is as extensive as ours, but it sounds like a good idea!

@furbrain
Copy link
Contributor Author

furbrain commented Jul 8, 2023

There's actually a fair bit of context held in the event loop, so I've just made it so that creating a new event loop clears all the previous references. I was also wondering if it was a deliberate decision to have a global _exc_context so that there weren't issues with creating a new object when memory might be tight

@dhalbert
Copy link
Contributor

dhalbert commented Jul 8, 2023

The intention is that only one event loop is supported, so maybe it was just a shortcut? I think you could ask this question in discussions (or make an issue) in the MicroPython rep. There is also an #asyncio channel in their discord.

@furbrain furbrain mentioned this issue Jul 8, 2023
@dhalbert
Copy link
Contributor

dhalbert commented Jul 8, 2023

OK, I looked at the code, and see what I said is overly obvious :) . Also I looked back at the blame history, and there's no churn in new_event_loop(): it is just as it was when new.

dhalbert added a commit that referenced this issue Jul 8, 2023
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

Successfully merging a pull request may close this issue.

2 participants