Skip to content
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

Fix memory deallocation #138

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix memory deallocation #138

wants to merge 8 commits into from

Conversation

oltarasenko
Copy link
Collaborator

  • Ensure proper string termination in get_function_sig
    by adding \0 for correct strlen usage.
  • Pass Proc struct to async_init to enable binary cleanup when
    the Port is closed.
  • Invoke hb_beamr:stop(WASM) in all tests to trigger garbage collection.
  • Free memory allocated for stub functions and remove stubs
    when the Port is closed.
  • Extend LoadWasmReq struct to store the list of stubs.

- Ensure proper string termination in `get_function_sig`
  by adding `\0` for correct `strlen` usage.
- Pass `Proc` struct to `async_init` to enable binary cleanup when
  the Port is closed.
- Invoke `hb_beamr:stop(WASM)` in all tests to trigger garbage collection.
- Free memory allocated for stub functions and remove stubs
   when the Port is closed.
- Extend `LoadWasmReq` struct to store the list of stubs.
Prior to this change, deallocating current_args in hb_beamr could lead
to segfaults due to uninitialized fields in the allocated structure.
This commit initializes the Proc struct with NULL values, ensuring safe
deallocation of resources.
@oltarasenko
Copy link
Collaborator Author

Hi @samcamwilliams,

I have made quite a few modifications to how we allocate and deallocate resources. Besides this, I have made a test that helps visualize how resources leak when a given number of iterations is done.

In the tests below I investigate the driver allocation memory when we perform 1M requests to WASM runtime. In both cases nodes have some basic memory usage visible in Activity Monitor (and it's approximately 130Mb).

Before the fix (main)
memory_consumption_before_fix_1M_requests

In the result, we see that the node started to consume 2.79 GB of RAM.

After the fix (current branch)
memory_consumption_after_fix_1M_requests

In the result, we see that the node started to consume 460 Mb of RAM.

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 this pull request may close these issues.

2 participants