Fix memory leak in get_map and get_map_bytes functions.

[JaatadiaMulesoft]

No description provided.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[PiotrSikora]

Hi! Thanks for finding the memory leak and providing this fix... Do you have a reproducer and/or test that we could add to the CI to catch this class of issues in the future?

[JaatadiaMulesoft]

I was testing compiling the filter and deploying it, after some request you can clearly see that the memory starts to grow. Now checking If I can add a unit test, I've found several libraries that could be used for this mockalloc and allocation_counter, etc. Any suggestion?

[JaatadiaMulesoft]

Test added

[PiotrSikora]

I was testing compiling the filter and deploying it, after some request you can clearly see that the memory starts to grow. Now checking If I can add a unit test, I've found several libraries that could be used for this mockalloc and allocation_counter, etc. Any suggestion?

Apologies for the delay.

The mockalloc test works fine for this particular case, but I was looking for a more generic solution for finding memory leaks (e.g. running sanitizers) using existing tests and/or examples.

Notably, #287 also incorrectly uses std::slice::from_raw_parts(), but one leak wouldn't be found with changes in this PR, and generally speaking, I'm not a big fan of using mocks instead of integration tests, so I think it would be better if we could run this against a real host implementation... and if we want to use mocks anyway, then it should cover more than a single function.

Having said that, that's a much bigger undertaking, and it shouldn't block this fix, so could you revert this PR to a version without the mockalloc? Sorry for the noise, I should have merged it right away without blocking it on tests.

[joesbigidea]

Hi! I notice this has been waiting for a while and I'm interested in this fix as well. I don't want to steal the thunder for this fix but would be happy to open a new PR with just the fix and without the additional test if that helps get this through.

[PiotrSikora]

@joesbigidea thanks for the nudge, this merge is way overdue.

I'll merge it as-is once #327 is merged to fix the CI.

[PiotrSikora]

Thanks!

[PiotrSikora]

@mpwarres could you take a look as well? I've made some small changes, and I don't want to self-approve.

[mpwarres]

@mpwarres could you take a look as well? I've made some small changes, and I don't want to self-approve.

Ack, taking a look