Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nil calls causing agents to become stranded #59

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

nil calls causing agents to become stranded #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75f3ede
Merge pull request #2 from adhearsion/develop
system123 Nov 1, 2016
39b2094
Fixed nil calls in queue causing agents to become stranded when using…
Nov 1, 2016
89db809
Connect callback is trigered before the call is actually connected
Nov 16, 2016
7acad81
Moved notification out of handler
Nov 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# [develop](https://github.com/adhearsion/electric_slide)
* Bugfix: Prevent nil objects in the call queue from crashing on connection
* Added `ElectricSlide::CallQueue#update_agent` to safely update a queued agent object's attributes
* Added `ElectricSlide::Agent#update` to update an agent object's attributes
* Added `ElectricSlide::Agent#callable?` to check if an agent can be called
Expand Down
8 changes: 6 additions & 2 deletions lib/electric_slide/call_queue.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ def remove_agent(agent, extra_params = {})
# Checks to see if any callers are waiting for an agent and attempts to connect them to
# an available agent
def check_for_connections
# Ensure there are no nil objects in the call queue before trying to connect
@queue.compact!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any theory as to how objects in the queue became nil? This seems like a bandaid, and probably incomplete?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce the issue on a small scale, only when we have more than 50 agents. The outbound call is valid and active when it is queued but it seems that by the time the loop checking for connections pops that call off the queue the call is then nil. My only thought is that the call dies and becomes nil somewhere between it being queued and getting connection, yet the connection is happening before the call is removed.

My thinking for this is that call_waiting? returns true still meaning the queue has a nil object on it, rather than it being empty. I have been battling to find the exact cause. This bandaid does solve the issue, but I agree it is not an ideal solution.

Copy link
Member

@bklang bklang Nov 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only thought is that the call dies and becomes nil somewhere between it being queued and getting connection, yet the connection is happening before the call is removed.

The thing is that values in an array can't become nil. The call can end, the actor can die, the value can become useless, but the value itself can't magically change from Adhearsion::Call to nil.

I can only think of two ways that a nil object would be enqueued:

  1. It enters the queue that way from the application - perhaps adding logging to the code that adds things to the queue may reveal the source?
  2. There is a yet-to-be-discovered bug in ElectricSlide that puts a nil into the queue - perhaps this can happen when a call connection fails? Maybe a call variable gets cleared and a nil value is re-added to the queue for the next agent? Adding trace logging to the code that puts a failed-attempt call back into the queue array could reveal this

connect checkout_agent, get_next_caller while call_waiting? && agent_available?
end

Expand Down Expand Up @@ -290,6 +292,8 @@ def remove_call(call)
def connect(agent, queued_call)
unless queued_call && queued_call.active?
logger.warn "Inactive queued call found in #connect"
agent.callback :connection_failed, current_actor, agent.call, queued_call

return_agent agent
return
end
Expand Down Expand Up @@ -446,13 +450,13 @@ def bridge_agent(agent, queued_call)
agent.call[:queued_call] = nil if agent.call
end

queued_call.register_tmp_handler :event, Punchblock::Event::Joined do |event|
queued_call.register_tmp_handler :event, Punchblock::Event::Joined do |event|
queued_call[:electric_slide_connected_at] = event.timestamp
end

agent.join queued_call if queued_call.active?
agent.callback :connect, current_actor, agent.call, queued_call

agent.join queued_call if queued_call.active?
rescue *ENDED_CALL_EXCEPTIONS
ignoring_ended_calls do
if agent.call && agent.call.active?
Expand Down