[kamailio/kamailio] Add fix_actions function parameter in fix_actions to allow call of free_fixup functions and free resources (PR #4023)
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [x] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [x] Related to issue #3814
#### Description <!-- Describe your changes in detail --> When the fixup is done in main using fix_rls function (fixing the route lists in cfg file), all the children inherit the memory allocated as a copy that needs separate management.
The function responsible for the fixup (fix_actions) is also used to call the free_fixup function with an added argument.
Then on main, we call the free_rls ( as fix_rls) on two separate occasions. Once in cleanup() function for the main process and once in the sig_usr() function for each of the children.
This seems to work fine and after debugging with `file_out` fixups, I can no longer see memory leaks due to the malloced memory in one of the fixups.
- main.c: Call free_rls() in cleanup() function to clean route lists and fixed up parameters in main process.
- main.c: Call free_rls() in sig_usr() function to clean route lists and fixed up parameters in child processes.
My question is, after a parameter is fixed, is it saved somewhere else, therefore we can immediately free the fix-up resources and not on shutdown?
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4023
-- Commit Summary --
* core: Add fix_actions function parameter in fix_actions to free fixed up resources
-- File Changes --
M src/core/route.c (61) M src/core/route.h (3) M src/core/rvalue.c (2) M src/core/switch.c (4) M src/main.c (4)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4023.patch https://github.com/kamailio/kamailio/pull/4023.diff
First, using `free` as a variable/parameter name is strongly not recommended, many code analyzers started to report such cases, it may result in unexpected behaviour for future changes, by hiding the function.
Them if it is about calling free fixup for `f1("$var(x)....")`, that's by design, to avoid parsing parameter again and again at runtime. If not, can you provided a more specific example when you need it, I looked again over the related reported issue, but I couldn't get where it was left that you needed a fix for.
@miconda Ahh true about `free`. I will change it to a more sensible one.
I do understand that for performance is best not to parse it again and again, so it must probably not be freed until the end of the shutdown.
The issue was that `free_fixup` function was not called for quoted parameters (ie `func1("$var(x)$var(y)");`, that have been fixed in startup, not even at shutdown.
Shouldn't the memory be freed as well or am i missing something?
@xkaraman pushed 1 commit.
e597615025159604c1aee496267e757d087c8dd5 core: Add fix_actions function parameter in fix_actions to free fixed up resources
The fixup result is in private memory, the clean up on signal/shut down is done only by main process, so it is rather useless, as the main process barely get the chance to execute config file. The fixup-free was mostly intended for dynamic use of the functions (e.g., from embedded interpreters, even before KEMI; or when the parameter is an expression).
Actually chunk-by-chunk cleaning at shut down, even for shared memory, can cause more troubles than benefits (e.g., a module cleans a structure that is referenced in another structure from another module). It is something I mentioned during past devel discussions and it is planned to be addressed also next week at Kamailio Devel Meeting.
Closed #4023.
As this was discussed in the Developer Meeting, it can be closed.
-
Daniel-Constantin Mierla -
Xenofon Karamanos