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

Wed Apr 9 16:08:42 CDT 2014

> On March 28, 2014, 12:27 p.m., Corey Farrell wrote:
> > Do we want to produce a warning if someone tries writing to refs from logger.conf (an error if they do so with REF_DEBUG enabled)?  I know this is unlikely, I'm just trying to think of everything since this is being applied to existing branches.
> > 
> > 
> > With refcounter.py I found two backtraces:
> > KeyboardInterrupt (Ctrl-C)
> > IOError: [Errno 32] Broken pipe (exiting less before refcounter.py finished output)
> > 
> > It'd be awesome if these would just silently exit.

Having logger.c emit an error if someone creates a log file with the name of 'refs': this isn't done for the memory allocation log file either. In general, if you're using REF_DEBUG, MALLOC_DEBUG, or other compile time options to create additional debug output, you should have some consideration for knowing what you're doing.

I added a try/except around the bulk of the processing that handles KeybaordInterrupt/SystemExit.

> On March 28, 2014, 12:27 p.m., Corey Farrell wrote:
> > /branches/1.8/channels/chan_sip.c, line 232
> > <https://reviewboard.asterisk.org/r/3377/diff/4/?file=56827#file56827line232>
> >
> >     Another comment like this was added to channels/sip/security_events.c in 11+.  Please make sure it's removed when you commit.

That's fine.

> On March 28, 2014, 12:27 p.m., Corey Farrell wrote:
> > /branches/1.8/contrib/scripts/refcounter.py, lines 130-133
> > <https://reviewboard.asterisk.org/r/3377/diff/4/?file=56828#file56828line130>
> >
> >     I don't much care for output of normal objects by default.  This produces very large output even for just starting and stopping asterisk (9.4MB in my test run of 1.8), and objects that didn't leak or skew are not normally useful.  If I were troubleshooting an issue and felt the need to see normal objects, I would just want the original /var/log/asterisk/refs to see the order of all refs without grouping by object.
> >     
> >     I'm not sure the other --suppress options are useful either.  I think it would be fine if we always print all skew and leak objects (both are errors), and give an option to enable showing of normal objects.  Less is better when it comes time to instruct a bug reporter on how to provide us the required information.

I don't really like having some options that invert output, and others that enable it. I'd rather either show all and disable what you don't want to show; or show none and enable what you want to show.

I prefer the suppression route myself. For now, I'll leave this as is.

- Matt

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

On April 9, 2014, 4:08 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3377/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 4:08 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/utils.xml 410891 
>   /branches/1.8/utils/refcounter.c 410891 
>   /branches/1.8/utils/Makefile 410891 
>   /branches/1.8/main/utils.c 410891 
>   /branches/1.8/main/logger.c 410891 
>   /branches/1.8/main/astobj2.c 410891 
>   /branches/1.8/main/asterisk.c 410891 
>   /branches/1.8/include/asterisk/utils.h 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/20140409/06aeebdd/attachment.html>