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

cfgutils route_if_exists causing leak #1503

Closed
jchavanton opened this issue Apr 19, 2018 · 4 comments
Closed

cfgutils route_if_exists causing leak #1503

jchavanton opened this issue Apr 19, 2018 · 4 comments

Comments

@jchavanton
Copy link
Member

jchavanton commented Apr 19, 2018

Description

cfgutils route_if_exists can leak when
1: using a $var or $avp
2: some functions are called within the called route

Troubleshooting

Reproduction

reproduced on both Kamailio 4.4.x and 5.x

Note that in my tests the route is executed from rtimer in case this is part of the problem somehow.

modparam("rtimer", "timer", "name=tmetrics;interval=9;mode=1;")
modparam("rtimer", "exec", "timer=tmetrics;route=test")

example :

route[dummy] {
        xinfo("[dummy]");
        statsd_gauge("dummy_1xx", $stat(1xx_replies));
}
route[test] {
        $var(route_name) = "dummy";
        route_if_exists($var(route_name));
]

working alternative

route[dummy] {
        xinfo("[dummy]");
        statsd_gauge("dummy_1xx", $stat(1xx_replies));
}
route[test] {
        $var(route_name) = "dummy";
        if (check_route_exists($var(route_name))) {
                     route($var(route_name));
       } 
}

Debugging Data

kamcmd pkg.stats you can see the `real_used` of the timer process with leak every time the route is called

reproduced on both Kamailio 4.4.x and 5.x

@jchavanton
Copy link
Member Author

Not sure if I will be able to find time for this one soon, since there is a working alternative this is probably low priority.

miconda added a commit that referenced this issue Apr 27, 2018
- do not run the route block as a top route
- related to GH #1503
@miconda
Copy link
Member

miconda commented Apr 27, 2018

It's rather impossible that $var(...) is leaking, but be aware that its value persists across executions of route blocks, the value is reset only by a new assign.

Then $avp() is using shared memory, so not related to pkg.stats output.

I looked at the code for route_if_exists and I discovered that was executing the route block as a top route, not as a sub-route (what route(x) does). I pushed a patch for it. Maybe that was the cause of leak, eventually inside the statsd operation, because route_if_exists doesn't allocate anything itself.

Update: statsd seems also ok, after a quick look, so it might be something else, anyhow, maybe you can test with the patch referenced above and see if ok now.

@jchavanton
Copy link
Member Author

right, it must be a side effect of having a variable param

kamcmd ps | grep met
2453	RTIMER EXEC child=0 timer=tmetrics
kamcmd pkg.stats | grep -A 5 2453 | grep real
	real_used: 2243864
kamcmd pkg.stats | grep -A 5 2453 | grep real
	real_used: 2244552

no luck with the patch ...

miconda added a commit that referenced this issue May 9, 2018
- do not run the route block as a top route
- related to GH #1503

(cherry picked from commit 9670bd8)
@miconda
Copy link
Member

miconda commented Jun 4, 2018

I pushed the above patch to have a similar behaviour for route_if_exists() like for route(). If the issue still exists, maybe it is better to open a new issue to track it in the context of the latest code.

@miconda miconda closed this as completed Jun 4, 2018
miconda added a commit that referenced this issue Jun 21, 2018
- do not run the route block as a top route
- related to GH #1503

(cherry picked from commit 9670bd8)
(cherry picked from commit cefd474)
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

No branches or pull requests

2 participants