[asterisk-dev] [Code Review] 3377: ref count logs: Redo structure of log file, provide a python debugging tool

[Corey Farrell]

> On March 28, 2014, 2:42 p.m., Corey Farrell wrote:
> > /branches/1.8/main/astobj2.c, line 238
> > <https://reviewboard.asterisk.org/r/3377/diff/4/?file=56831#file56831line238>
> >
> >     Can we add thread id to all reflog entries (before tag so it is parsable)?  ast_get_tid() was added to utils.c in Asterisk 11.  Asterisk 1.8 has an inferior macro in logger.c that could probably just be copied.  This would allow reflog entries to be associated with logger.c output.  If a leak is associated with a specific channel we could see which one, examine traffic from PCAP, etc.
> >     
> >     I don't think we need an option for refcounter.py to filter by thread id, grep can do that.  In most situations where thread id would be useful we would want the original refs file and full debug logs from the same run.
> 
> Matt Jordan wrote:
>     I'm sitting in an airport right now trying to debug ref counting problems. I'll take a look at adding this.
>     
>     At the same time, this review has had more findings and more enhancements than reviews for the bugs it seeks to fix. Unless something is critically necessary or is a finding against the actual functionality in this patch, I'm going to ask that we put it in another patch/review.

It's your call if you want to drop this enhancement.  I was originally thinking that thread id was just something crazy that I added, but then I'm realizing it's not actually crazy at all given how much can be learned from it.  I opened a finding against this since we're already breaking the format of the refs log, I assumed that breaking it once was best.

I understand if you don't want this change to deal with the fact that ast_get_tid() is not available in all supported versions.  If you reserve a field (set to 0), this would allow me to submit a review to add TID later without breaking the format again.  If you do that I would say fix the array indexes in refcounter.py, but don't output the reserved field.


- Corey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3377/#review11435
-----------------------------------------------------------


On March 27, 2014, 10:23 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3377/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 10:23 p.m.)
> 
> 
> Review request for Asterisk Developers, Corey Farrell and wdoekes.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Note: while an improvement to Asterisk, this patch only affects Asterisk when compiled in -dev-mode. Since it has benefit only for developers looking to fix bugs, I'm proposing it for Asterisk 1.8+. The removal of refcounter should be done in trunk only.
> 
> Asterisk uses reference counted objects. A lot. As their use has spread, the bugs related to reference counting errors has grown as well. Central to resolving reference counting issues is the usage of REF_DEBUG; unfortunately, REF_DEBUG has had several problems:
> (1) It could not be enabled through menuselect
> (2) When not enabled globally, updates to objects were often lost, resulting in insufficient data to resolve problems
> (3) The format of the ref debug log file was not suitable for parsing
> 
> This patch changes REF_DEBUG in the following ways:
> (1) It makes REF_DEBUG a meneselect item when Asterisk is compiled with --enable-dev-mode
> (2) The ref debug log file is now created in the AST_LOG_DIR directory. Every run will now blow away the previous run (as large ref files sometimes caused issues). We now also no longer open/close the file on each write, instead relying on fflush to make sure data gets written to the file (in case the ao2 call being performed is about to cause a crash)
> (3) It goes with a comma delineated format for the ref debug file. This makes parsing much easier.
> (4) It throws out the refcounter utility and goes with a python script instead.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/utils/refcounter.c 410891 
>   /branches/1.8/utils/Makefile 410891 
>   /branches/1.8/main/astobj2.c 410891 
>   /branches/1.8/main/asterisk.c 410891 
>   /branches/1.8/include/asterisk/astobj2.h 410891 
>   /branches/1.8/contrib/scripts/refcounter.py PRE-CREATION 
>   /branches/1.8/channels/chan_sip.c 410891 
>   /branches/1.8/build_tools/cflags.xml 410891 
> 
> Diff: https://reviewboard.asterisk.org/r/3377/diff/
> 
> 
> Testing
> -------
> 
> Things spit out ref logs and the script parses them. Example below:
> 
> ==== Object 0x21ed9b8 history ====
> features.c[5427]:create_parkinglot 1  - [**constructor**]
> features.c[5707]:build_parkinglot +1  - [1]
> features.c[5392]:parkinglot_unref -1  - [2]
> features.c[6358]:build_dialplan_useage_map +1  - [1]
> features.c[6358]:build_dialplan_useage_map -1  - [2]
> features.c[4985]:find_parkinglot +1  - [1]
> features.c[5392]:parkinglot_unref -1  - [2]
> features.c[6358]:build_dialplan_useage_map +1  - [1]
> features.c[6358]:build_dialplan_useage_map -1  - [2]
> features.c[4955]:do_parking_thread +1  - [1]
> features.c[4957]:do_parking_thread -1  - [2]
> astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [1]
> astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [**call destructor**]
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140406/565b90eb/attachment-0001.html>