[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:              09-12-2007 05:41 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.
====================================================================== 

---------------------------------------------------------------------- 
 arkadia - 09-12-07 05:41  
---------------------------------------------------------------------- 
I briefly check CDRfix5.

Imagine you have dialplan with Dial(). How are you going to set 
Set(CDR_CONTROL(${mycdr})=answer) at the same moment when cannel was
answered in Dial? How can it be closed if call is finished in Dial().


Set(mycdr=${CDR_CONTROL(start)}). 'mycdr' by default should behave like
its parent CDR. Othervise it'll be too much work in dialplan to take care
about every CDR field. E.g. I don't want to spend time setting 'lastapp'
after each application call.


In your example:
"                Set(mycdr=${CDR_CONTROL(start)});\n"
"                Set(CDR_CONTROL(${mycdr})=answer);\n"
"                Set(CDR(lastapp,,${mycdr})=Record+BackGround);\n"
"                Set(CDR(lastdata,,${mycdr})=whoknows);\n"
"                Record(recording:wav);\n"
"                Background(recording);\n"
"                Set(CDR_CONTROL(${mycdr})=close);\n"
CDR fields in dialplan updated one by one. We should remember that call
may be interrupted at any time and next Set() may never be executed. It
opens possibilities for all kind of race conditions. At least all
assignments should be done by one Set(CDR()=....,CDR()=...,)


"Every CDR created with CDR_CONTROL() should be matched at some point with
a 'close' or 'abort' action to avoid memory leaks." Call may be interrupted
at any moment, so I expect "Set(CDR_CONTROL(${mycdr})=close)" won't be
reached all the time.


I don't see how 'mycdr' data going to be saved. Are you going to post it
together with regular CDR through every supported CDR engine?


As I understand CDR_CONTROL actually controls not a CDR but a set of
variables under 'handle'. This kind of behavior can be implemented using
current custom CDR variables. Like:
Set(CDR(mycdr1_disposition)=answer)
Set(CDR(mycdr1_lastapp)=Record+BackGround)
Set(CDR(mycdr1_lastdata)=whoknows)
Or I miss something? 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
09-12-07 05:41  arkadia        Note Added: 0070388                          
======================================================================