| Summary: | ASTERISK-01703: Queue application does not properly update lastcall time | ||
| Reporter: | moorejon (moorejon) | Labels: | |
| Date Opened: | 2004-05-26 21:51:34 | Date Closed: | 2008-01-15 14:56:51.000-0600 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Applications/app_queue |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) app_queue.c ( 1) app_queue.c.diff | |
| Description: | The queue member structure documents the lastcall value as storing the value of the time of hangup of the last queue call taken by the queue member. In actuality the value stores the start time of the last call. ****** ADDITIONAL INFORMATION ****** Adding the following line at 850 of app_queue.c fixes the bug. Don't know if it causes any side effects. Implmented this to work with my other bug report regarding wrapuptimers for dynamic queue members. time(&lpeer->member->lastcall); Attached file is against 0.7.2 | ||
| Comments: | By: Brian West (bkw918) 2004-05-26 22:07:09 please attach a diff -u and tell us what verison this is against! By: moorejon (moorejon) 2004-05-26 23:31:30 This is against stable branch 0.7.2 from my test system. Made same modification to a production 0.9.0 system and seems to also work there. By: Brian West (bkw918) 2004-05-26 23:48:33 please try -stable or -head to see if the problem is still there. Might have already been fixed. bkw By: moorejon (moorejon) 2004-05-27 09:45:47 I will download and check, but I believe it is still there. I reviewed all the ACD bug reports before beginning changes and there is no mention of this problem anywhere else. Would you like a diff agains -stable or -head? By: Brian West (bkw918) 2004-05-27 12:23:50 well I use head and mine updates fine.. can you explain a bit more of why you think it doesn't update correctly? By: moorejon (moorejon) 2004-05-27 12:57:08 Yes, it updates but the update time is set at the start of the call. If you look at the comments for the member data structure it indicates that the lastcall is supposed to be set at the time of hangup of the last call. If you try to implement wrapuptimers with the time set at the start of the last call there is no way to know how long ago the call hung up. If you reset the value at the end of the call then you know how long since the lastcall ended. So it is a question of whether the lastcall value is supposed to store the time off the start of the last call or the end. By: Mark Spencer (markster) 2004-05-28 12:06:50 Unfortunately, "lpeer" has been freed by the time your patch updates it, meaning that's an invalid memory access and will eventually cause you a segfault. It makes the most sense to have lastcall end on hangup (although if someone has a reasonable excuse for the first, i'm happy to add a new strategy). By: moorejon (moorejon) 2004-05-28 13:12:00 Maybe I am being dense, but can you point out where lpeer is released? I don't see any code that does anything to lpeer between where it is referenced by update_queue at around line 808. I don't see why there would be any reason to release lpeer since it is a pointer to the queue member and the member is still in the queue between calls. lpeer is created as a pointer in this function, so the pointer storage wouldn't end until the end of the sub routine. The queue member it is pointed at isn't going away either, so I don't understand what would be released prior to my access. Please explain. By: Mark Spencer (markster) 2004-05-30 16:28:24 it's done when we call hanguptree... lpeer is one of those. I'll try to fix it and let ya test out my fix. By: Mark Spencer (markster) 2004-05-30 17:55:00 Okay check out latest CVS head, it should fix this, please confirm that works for you as soon as you can. thanks! By: moorejon (moorejon) 2004-05-31 16:00:08 Thanks for the update. Haven't had a chance to load it on my test box yet, but reviewed diff against previous head version, and I am wondering if this introduces a new bug? Looks like you created a member pointer to use instead of lpeer and then call update_queue once just prior to starting the call and then once after ending it. I think this increments the call count twice for the member for each call instead of just once. By: Mark Spencer (markster) 2004-05-31 19:28:27 Okay try it now and let me know. Presumably it wouldn't cause a problem since everyone was incrementing by two, but this is probably the "right way". By: Olle Johansson (oej) 2004-06-13 15:46:17 This bug report needs feeback - did Markster's changes solve this issue? /Housekeeping :-) By: Mark Spencer (markster) 2004-06-18 23:40:04 Presumed fixed By: Digium Subversion (svnbot) 2008-01-15 14:56:51.000-0600 Repository: asterisk Revision: 3114 U trunk/apps/app_queue.c ------------------------------------------------------------------------ r3114 | markster | 2008-01-15 14:56:51 -0600 (Tue, 15 Jan 2008) | 2 lines Update queue member after end of call (bug ASTERISK-1703) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=3114 | ||