[asterisk-bugs] [Asterisk 0010668]: Incomplete CDR lock

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=10668 
====================================================================== 
Reported By:                arkadia
Assigned To:                murf
====================================================================== 
Project:                    Asterisk
Issue ID:                   10668
Category:                   CDR/General
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
Asterisk Version:           1.4.11  
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!):  
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             09-07-2007 08:26 CDT
Last Modified:              06-04-2008 12:33 CDT
====================================================================== 
Summary:                    Incomplete CDR lock
Description: 
This is all about the meaning of AST_CDR_FLAG_LOCKED flag for CDR. 
I assume if CDR has this flag set then it wont be updated anymore by any
ast_cdr_ functions except cases when we force such update. ast_cdr_reset()
is example of function which allows to update locked CDRs if special flag
is given.

Based on this assumption I prepared the patch with such changes:

  ast_cdr_setvar() - not allowed to touch locked CDRs. I don't see any
cases when we need to change locked CDRs too. Not touching locked CDRs is
helpfull when ForkCDR being used and every new CDR may have different
custom variables.

  ast_cdr_answer() - don't change status of locked CDRs. That might be
helpfull when we have a chain of CDRs for one incoming call for which we
perform several tries to make outgoing call. Every outgoing attempt may
finish with its own completion code. And if the final attempt is 'answered'
then we have only one CDR with 'answered' status. All other will keep
connection error codes which is very helpful for error monitoring.

 ast_cdr_end() - don't change locked CDRs by the same reason as
ast_cdr_answer()

As there will be no changes in CDR after lock, app_forkcdr.c should be
updated to end original locked CDR.
Also it'll be great to make optional cdr reset in this application, but
thats a theme for another patch.
======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
related to          0012726 ForkCDR application fails to set duration
====================================================================== 

---------------------------------------------------------------------- 
 murf - 06-04-08 12:33  
---------------------------------------------------------------------- 
The "e" option on forkCDR won't let you end the previous CDR? Won't that
give you what you need?  Hmmm. Let's see... it would, but if it isn't
immediately posted, then you get the "answered" status upgraded to
ANSWERED"
when a following Dial() does succeed... Hmmm.

So, I did my research. cdr_end() is OK, because it won't overwrite a
previously
set non-zero timestamp.

cdr_answer() will set every cdr with disp < ANSWERED to ANSWERED.

setvar will set the var on every CDR in the chain, if recur is set in the
options. So I checked the CDR() func. and there's a recur option (r) and a
last (l) option. the cdr write func doesn't obey the (l) flag, but that's a
bug we can fix.

So, can I close this bug, if I:

(1) I can provide one more option ('A') to forkCDR that will prevent the
answer time from being updated in LOCKED cdrs. (I'll use something like
ANSWERLOCK.)

(2) Update the CDR() write func to obey the (l) flag. I'd think that this
a true bug according to the spec, the doc string, and common sense. The
only possible hitch might be if people are depending on this bug; then they
will have to realize that it is not good to depend on behavior that is the
result of a bug.

?

I can/will add this 1.4 and to trunk, and 1.6.0. I'll still consider it a
bug fix, as the addition will not hurt current users, and the sparse
documentation around forkCDR would allow your interpretation, given some
slack. 

The only possible hitch might be when I fix CDR() to obey the (l) flag on
writing. If people are depending on that, then they will have to realize
that it is not good to depend on behavior that is the result of a bug. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
06-04-08 12:33  murf           Note Added: 0087816                          
======================================================================