[Home]

Summary:ASTERISK-12304: [patch] app_queue does not handle Custom: state interfaces
Reporter:Alexandru Pirvulescu (sigxcpu)Labels:
Date Opened:2008-07-03 04:36:17Date Closed:2008-09-09 12:06:02
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 12979.patch
( 1) 20080710_issue12979_queue_custom_state_interface_16branch_2.diff
( 2) 20080710_issue12979_queue_custom_state_interface_16branch_3.diff
( 3) 20080710_issue12979_queue_custom_state_interface_16branch.diff
( 4) 20080710_issue12979_queue_custom_state_interface_trunk_2.diff
( 5) 20080710_issue12979_queue_custom_state_interface.diff
Description:AddQueueMember(1000,Local/1000@agents/n,,,,Custom:1000);

This should treat Custom:1000 as a state interface for this member. Unfortunately, handle_statechange() in app_queue.c handles only interfaces with "technology", containing a "/" in their name, that is.

From what I've seen, there are two lookups in the agent interfaces list, one in handle_statechange() and the other one in update_status(). I've removed the first one and I came up with a simpler form of handle_statechange. This just passes the state interface name to the update_status. I've also removed "/" checking in update_status(), so now Custom: state interfaces are working 100%.

The real application I'm using is emulation of chan_agent with MeetMe rooms, as AgentLogin does not have dynamic capabilities and I need "live" agents (always on).

Comments:By: Brett Bryant (bbryant) 2008-07-08 17:42:21

sigxcpu, we can only review your code after you've signed a license agreement with digium and uploaded it as a file.
By: Alexandru Pirvulescu (sigxcpu) 2008-07-09 04:57:31

Hi, the code was put there just for an idea. There is not patch submittal, it's just a bug notification with additional information for a developer to know what to look at.
By: Brett Bryant (bbryant) 2008-07-10 12:49:42

sigxcpu, could you test one of the patches i've uploaded to see if it fixes your problem? There are two patches, one for trunk and one for 1.6.
By: Joel Vandal (jvandal) 2008-07-10 13:01:28

The fix i've post on 13043 is already implemented/included on your patch.

013043: [patch] Add missing state_interface on the dump_queue_members function

But 13043 will also apply to 1.4/1.6/trunk.
By: Brett Bryant (bbryant) 2008-07-10 14:12:02

Ok, thanks jvandal, I went ahead and committed your change to trunk and 1.6 and removed it from these patches.
By: Alexandru Pirvulescu (sigxcpu) 2008-07-10 15:51:45

Nope. the 16branch patch does not work with 1.6.0-beta9. It crashes Asterisk as below:

*** glibc detected *** asterisk: double free or corruption (fasttop): 0x08247248 ***
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6[0xb7e1da85]
/lib/tls/i686/cmov/libc.so.6(cfree+0x90)[0xb7e214f0]
/usr/lib/asterisk/modules/app_queue.so[0xb6eca684]
asterisk[0x812d66b]
/lib/tls/i686/cmov/libpthread.so.0[0xb7d9a4fb]
/lib/tls/i686/cmov/libc.so.6(clone+0x5e)[0xb7e88e5e]
By: Brett Bryant (bbryant) 2008-07-10 16:08:56

sigxcpu, that's been fixed in the new patch.
By: Alexandru Pirvulescu (sigxcpu) 2008-07-10 16:53:06

Nope, the _2 version is not working, neither. Now it is not crashing, but the agent state is Not In Use, regardless the status of the stateinterface (like the stock 1.6.0-beta9).

Changed eventmemberstatus to no, but still no luck (I've seen that maskmemberstatus should be unset, to update the status and it depends on eventmemberstatus in configuration file).

I've removed the maskmemberstatus checking and it is working.

I think the status should be updated regardless of the maskmemberstatus value. As far as I've read, this value should just inhibit the Manager event.


By: Brett Bryant (bbryant) 2008-07-11 11:47:43

sigxcpu, right, that should be fixed in the newest patch, thanks for testing.
By: snuffy (snuffy) 2008-08-22 10:17:25

Sigxcpu does the final patches bbryant uploaded now exhibit the correct behavior?
By: Alexandru Pirvulescu (sigxcpu) 2008-08-23 02:43:44

Yes, it does. Sorry for not confirming.
By: Alexandru Pirvulescu (sigxcpu) 2008-09-04 09:51:33

Hi, sorry for coming back again. I've downloaded today 1.6.0-rc4 and, looking at the sources, this bug is there.
Maybe I don't understand how the release system works, but shouldn't this be fixed in this version?
By: Mark Michelson (mmichelson) 2008-09-05 17:14:06

sigxcpu: Sorry about the delay, the patch is still going through the review process and hasn't actually gotten committed yet. I got notified of the patch today and have started making sure, architecturally, that it is all right.
By: Mark Michelson (mmichelson) 2008-09-09 06:37:04

I've taken a closer look at this, and while the patch should fix the issue, it does quite a bit more in addition. The patch provided here is definitely a candidate for trunk, but something of this magnitude should not be made to the 1.6.0 branch when it is so close to being officially released.

I will provide a trimmed-down patch that just fixes the issue at hand for testing.
By: Mark Michelson (mmichelson) 2008-09-09 07:34:32

I've uploaded 12979.patch. It is a minimal amount of change to allow the use of custom device states. Please test. As soon as I hear confirmation that it is working correctly, I will commit this to the 1.6.0 branch. Sorry about the confusion earlier.
By: Alexandru Pirvulescu (sigxcpu) 2008-09-09 10:45:01

Hi, it seems to be working. I've applied patch against 1.6.0-rc5.
I've tested few times and the agent goes INUSE / NOT_INUSE as expected.
By: Mark Michelson (mmichelson) 2008-09-09 11:15:06

Thank you very much, I'll commit and get this closed ASAP!
By: Digium Subversion (svnbot) 2008-09-09 11:17:31

Repository: asterisk
Revision: 142090

U   branches/1.6.0/apps/app_queue.c

------------------------------------------------------------------------
r142090 | mmichelson | 2008-09-09 11:17:30 -0500 (Tue, 09 Sep 2008) | 16 lines

Fix app_queue's device state callback so that it
can correctly parse custom device states (and any
other device which does not contain a '/').

1.6.1 will be getting this patch as well, but trunk
is going to get a much more massive patch by bbryant
which does some very nice overhauling of some
structures in app_queue.

(closes issue ASTERISK-12304)
Reported by: sigxcpu
Patches:
     12979.patch uploaded by putnopvut (license 60)
Tested by: sigxcpu


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=142090
By: Digium Subversion (svnbot) 2008-09-09 12:06:00

Repository: asterisk
Revision: 142146

U   trunk/apps/app_queue.c

------------------------------------------------------------------------
r142146 | mmichelson | 2008-09-09 12:05:59 -0500 (Tue, 09 Sep 2008) | 21 lines

This is the trunk version of the patch to close
issue 12979. The difference between this and the
1.6.0 and 1.6.1 versions is that this is a much more
invasive change. With this, we completely get rid
of the interfaces list, along with all its helper
functions.

Let me take a moment to say that this change personally
excites me since it may mean huge steps forward regarding
proper lock order in app_queue without having to strew
seemingly unnecessary locks all over the place. It also
results in a huge reduction in lines of code and complexity.
Way to go Brett!

(closes issue ASTERISK-12304)
Reported by: sigxcpu
Patches:
     20080710_issue12979_queue_custom_state_interface_trunk_2.diff uploaded by bbryant (license 36)
Tested by: sigxcpu, putnopvut


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=142146