[Home]

Summary:ASTERISK-12705: skinny memory leak
Reporter:pj (pj)Labels:
Date Opened:2008-09-10 04:31:44Date Closed:2008-09-14 17:07:11
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_skinny
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) memleak1.diff
( 1) memleak2.diff
( 2) memleak3.diff
( 3) memleak4.diff
( 4) memleak5.diff
( 5) skinny-mem-alloc.zip
( 6) skinny-mem-summ1.txt
( 7) skinny-mem-summ2.txt
Description:memory allocations continuously growing,
asterisk has only two idle 7920 phones during monitoring period,
no skinny calls during this time window (about 15hours),
perhaps some leak in keepalives handling?

 176148 bytes in 3284 allocations in file 'chan_skinny.c'
 307800 bytes in 7523 allocations in file 'chan_skinny.c'





Comments:By: Damien Wedhorn (wedhorn) 2008-09-10 18:17:20

Looks like we don't free req after it is transmitted. A simple free(req) at the end of transmit_response will probably fix most of the problem, although will need to check all the other transmit_ stuff that it doesn't return without calling transmit_response. Will have a play when I get home.
By: Damien Wedhorn (wedhorn) 2008-09-11 03:33:05

Uploaded memleak1 which fixes the req_alloc leak (simple). However, also noticed that subs are growing, we obviously need to destroy the subs at some stage.
By: Damien Wedhorn (wedhorn) 2008-09-11 04:02:20

memleak2 seems to fix both issues. Don't like the sub hangup stuff, we should do it all in a single spot. Tested on the old phones and seems to work and no memory leaks apparent.
By: pj (pj) 2008-09-11 13:00:52

I tried your patch and it seems, that memmory leaks are really gone, thanks!
Can you commit this changes or do you still working on it?

Can I have some unrelated question to this bugreport? every call (at beginning call phase) from 7920 phone generates this warning on cli:
[Sep 11 20:03:41] WARNING[11320]: chan_skinny.c:1563 find_subchannel_by_instance_reference: Could not find subchannel with reference '5' on 'PJ'
(reference value increases for every call)
should I wrote separate bugreport for this, or do you think, that it's so trivial, that can be fixed immediatelly?
It seems, that only 7920 phones generates this warnings, cisco 7912 phone is OK.


By: Damien Wedhorn (wedhorn) 2008-09-11 15:21:45

The can't find subchannel by reference is on my todo list. While deleting the message would be trivial, the issue is actually because skinny handles finding subs in two different ways (new and old). The old phones (30VIP and 12SP) have the same issue as the 7920. Other new phones shouldn't display that message.

That problem will be fixed as part of a bigger rewrite of the sub handling.
By: pj (pj) 2008-09-12 08:12:12

I'm using your patch memleak2.diff and observing some crashes after skinny hangup, please look at bugreport ASTERISK-1321294
maybe you will find something interesting in gdb or mmlog, that I posted there.
By: Michiel van Baak (mvanbaak) 2008-09-12 08:17:01

@pj: You see those crashes on a stock asterisk or only on asterisk patched with memleak2.diff ?
By: pj (pj) 2008-09-12 08:32:07

initially I created bugreport ASTERISK-1321294 before patch memleak2.diff was applied, but my last two messages there, was from crashes after I patched asterisk, and if you look at mmlog, something is wrong in req_alloc of chan_skinny, but I'm not sure, if last crashes can be related to this patch.
By: Damien Wedhorn (wedhorn) 2008-09-12 17:52:48

memleak3.diff Fixes at least one of the memory violations. Comments included in diff regarding issues in transmit_tone and transmit_displaymessage.

Also adds transmit debug so we can see where these issues are occuring. Sorry, at the moment the extra debugging is overbearing, but it is only for testing this issue (and the related). Will move it off to its on bug later and include config options.

Tested on my 30VIPs and not getting any high fence violations.

Edit: don't think that it'll help the other issue as we were overwriting our allocation by two uint_32's and the buffer is (I think) 8 bytes. So we would have just been violating the buffer rather than other structures.


By: pj (pj) 2008-09-13 01:13:28

Asterisk SVN-trunk-r142992 +  memleak3.diff
sometimes crashes (not easy reproducable) after skinny hangup (gdb attached),
mmlog:
WARNING: Freeing unused memory at 0x831a348, in skinny_hangup of chan_skinny.c, line 3690
WARNING: Freeing unused memory at 0x830c9b0, in skinny_hangup of chan_skinny.c, line 3690


(gdb) bt
#0  0xb7d8c26f in ?? () from /lib/i686/libc.so.6
#1  0xb7c3e9ab in _Unwind_Backtrace () from /lib/libgcc_s.so.1
#2  0xb7d8c428 in backtrace () from /lib/i686/libc.so.6
#3  0x080f8036 in ast_bt_get_addresses (bt=0xc250362c) at logger.c:1190
#4  0xb78c9783 in __ast_pthread_mutex_lock (filename=0xb78e0274 "chan_skinny.c", lineno=3680, func=0xb78e2a9e "skinny_hangup", mutex_name=0xb78e1731 "&sub->lock", t=0x82fa9e8)
   at /root/src/asterisk143030/include/asterisk/lock.h:501
ASTERISK-1  0xb78d2b7e in skinny_hangup (ast=0x82f8ab8) at chan_skinny.c:3680
ASTERISK-2  0x080955ea in ast_hangup (chan=0x82f8ab8) at channel.c:1623
ASTERISK-3  0x08114fc5 in __ast_pbx_run (c=0x82f8ab8) at pbx.c:3898
ASTERISK-4  0x08115bae in ast_pbx_run (c=0x82f8ab8) at pbx.c:4024
ASTERISK-5  0xb78d19a9 in skinny_newcall (data=0x82f8ab8) at chan_skinny.c:3446
ASTERISK-6 0xb78d1eab in skinny_ss (data=0x82f8ab8) at chan_skinny.c:3513
ASTERISK-7 0x0816d6f0 in dummy_start (data=0x82ca0f0) at utils.c:1024
ASTERISK-8 0xb7c8a315 in start_thread () from /lib/i686/libpthread.so.0
ASTERISK-9 0xb7d78dde in clone () from /lib/i686/libc.so.6


By: Damien Wedhorn (wedhorn) 2008-09-13 18:41:50

new patch (memleak4.diff). I was a bit overzealous with freeing the subs which appears to have caused some issues. Only a single ast_free(sub).

Unfortunately it's difficult to force a crash (only managed 1 - mvanbaak got 1 as well with memleak3.diff) so not certain that this actually fixes the issue.

pj, as it seems to be easiest for you to get crashes, can you please test. Not certain that memleak3 is related to your earlier crashes.

Edit: memleak4 excludes the extra debugging stuff.


By: Michiel van Baak (mvanbaak) 2008-09-14 03:20:56

+ ast_free(req);
+
if (res != letohl(req->len)+8) {


Is that correct ?
By: Damien Wedhorn (wedhorn) 2008-09-14 05:17:26

Nope, try memleak5, notice that it was in all my patches.
By: pj (pj) 2008-09-14 13:05:40

Asterisk SVN-trunk-r143034 + memleak5.diff
no more crashes today during my test calls,
no memory leaks or warning messages in mmlog,
I think it can be commited soon, thanks!
and if you think, that ASTERISK-1321294 could be also fixed by this patch, you can close also this bugreport.


By: Digium Subversion (svnbot) 2008-09-14 17:07:06

Repository: asterisk
Revision: 143082

U   trunk/channels/chan_skinny.c

------------------------------------------------------------------------
r143082 | mvanbaak | 2008-09-14 17:07:05 -0500 (Sun, 14 Sep 2008) | 11 lines

plug a couple of memleaks in chan_skinny.

(closes issue ASTERISK-12705)
Reported by: pj
Patches:
     memleak5.diff uploaded by wedhorn (license 30)
Tested by: wedhorn, pj, mvanbaak

(closes issue ASTERISK-12571)
Reported by: pj

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=143082