[asterisk-dev] [Code Review] Don't miss control frames if a call is answered and hung up very quickly

[Terry Wilson]

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

(Updated 2010-10-08 14:40:58.703056)


Review request for Asterisk Developers, Russell Bryant and David Vossel.


Changes
-------

The first thing __ast_queue_frame does is check if the last frame queued is a HANGUP, so it shouldn't be possible to queue frames after a hangup frame is sent. So, as long as we get a hangup, the readq will eventually be empty except for that frame. In __ast_read(), we will then see that frame and set f to NULL which means that we'll hit the else block where we set END_READ and also return NULL.  Now, after returning NULL, whoever is reading is supposed to stop reading. If __ast_read() is called again, we'll see that END_READ is set and immediately goto done.

If someone just sets _softhangup and they *never* queue a hangup (or null) frame, first they should be shot. Second, I suppose when we check ast_check_hangup() in __ast_read, we can just queue a null frame there for sport. ast_queue_frame will ignore it if a HANGUP has been queued anyway. If it hasn't, then we ensure that we'll stop at some point.

I've simplified the patch by only putting in the changes required to fix the issue. All of the checks for _softhangup == x can just stay the way they are for now, since it is really a separate issue.


Summary
-------

When an outgoing call is answered and hung up by the far end *very* quickly, we may not read any frames and therefor end up with a call that displays the wrong disposition/DIALSTATUS. The reason is because ast_queue_hangup() immediately sets the _softhangup flag on the channel and then queues the HANGUP control frame, but __ast_read refuses to read any frames if ast_check_hangup() indicates that a hangup request has been made (which it will if _softhangup is set). So, we end up losing control frames.

This change makes __ast_read continue to read frames even if a soft hangup has been requested. I believe this should be safe as we actually check farther down in __ast_read for HANGUP frames and return NULL just like we would have if we skipped the read via the goto done statement, and we don't call into any of the channel tech callbacks unless the readq is empty.


This addresses bug 16946.
    https://issues.asterisk.org/view.php?id=16946


Diffs (updated)
-----

  /branches/1.4/include/asterisk/channel.h 289024 
  /branches/1.4/main/channel.c 289024 

Diff: https://reviewboard.asterisk.org/r/740/diff


Testing
-------

I have verified via sipp that when placing an outbound call that is answered and immediately hung up that the call shows up as answered (where it doesn't w/o the patch). I have also run the testsuite and verified that all tests continue to pass.


Thanks,

Terry