From a29c99160b728b24c01f5ca616e2e872a1cb201b Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Thu, 31 Dec 2015 14:09:37 -0200 Subject: [PATCH 1/2] One should not hand out an Actor's internal reference This makes it impossible to set timers on the queue from Agent callbacks, and is generally not thread-safe. --- CHANGELOG.md | 1 + lib/electric_slide/call_queue.rb | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6ba55d..e3a2d3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ # [develop](https://github.com/adhearsion/electric_slide) + * Bugfix: Don't hand out internal CallQueue references to Agents - thread-safety # [0.4.0](https://github.com/adhearsion/electric_slide/compare/v0.3.0...v0.4.0) - [2015-12-17](https://rubygems.org/gems/adhearsion/versions/0.4.0) * Change `ElectricSlide::Agent` `#presence=` method name to `#update_presence`. It will also accept one more parameter called `extra_params`, a hash that holds application specific parameters that are passed to the presence change callback. diff --git a/lib/electric_slide/call_queue.rb b/lib/electric_slide/call_queue.rb index 5a47f95..a781a7c 100644 --- a/lib/electric_slide/call_queue.rb +++ b/lib/electric_slide/call_queue.rb @@ -164,7 +164,7 @@ def add_agent(agent) abort ArgumentError.new("#add_agent called with nil object") if agent.nil? abort DuplicateAgentError.new("Agent is already in the queue") if get_agent(agent.id) - agent.queue = self + agent.queue = current_actor case @connection_type when :call @@ -177,7 +177,7 @@ def add_agent(agent) @agents << agent @strategy << agent if agent.presence == :available # Fake the presence callback since this is a new agent - agent.callback :presence_change, self, agent.call, agent.presence, :unavailable + agent.callback :presence_change, current_actor, agent.call, agent.presence, :unavailable async.check_for_connections end @@ -304,7 +304,7 @@ def connect(agent, queued_call) ignoring_ended_calls do if agent.call && agent.call.active? logger.warn "Dead call exception in #connect but agent call still alive, reinserting into queue" - agent.callback :connection_failed, self, agent.call, queued_call + agent.callback :connection_failed, current_actor, agent.call, queued_call return_agent agent end @@ -367,8 +367,10 @@ def call_agent(agent, queued_call) # Stash the caller ID so we don't have to try to get it from a dead call object later queued_caller_id = remote_party queued_call + queue = current_actor + # The call controller is actually run by #dial, here we skip joining if we do not have one - dial_options = agent.dial_options_for(self, queued_call) + dial_options = agent.dial_options_for(queue, queued_call) unless dial_options[:confirm] agent_call.on_answer { ignoring_ended_calls { agent_call.join queued_call.uri if queued_call.active? } } end @@ -394,12 +396,12 @@ def call_agent(agent, queued_call) agent.call = nil - agent.callback :disconnect, self, agent_call, queued_call + agent.callback :disconnect, queue, agent_call, queued_call unless connected if queued_call.active? ignoring_ended_calls { priority_enqueue queued_call } - agent.callback :connection_failed, self, agent_call, queued_call + agent.callback :connection_failed, queue, agent_call, queued_call logger.warn "Call did not connect to agent! Agent #{agent.id} call ended with #{end_event.reason}; reinserting caller #{queued_caller_id} into queue" else @@ -408,7 +410,7 @@ def call_agent(agent, queued_call) end end - agent.callback :connect, self, agent_call, queued_call + agent.callback :connect, queue, agent_call, queued_call agent_call.execute_controller_or_router_on_answer dial_options.delete(:confirm), dial_options.delete(:confirm_metadata) @@ -420,8 +422,9 @@ def bridge_agent(agent, queued_call) queued_caller_id = remote_party queued_call agent.call[:queued_call] = queued_call + queue = current_actor agent.call.register_tmp_handler :event, Punchblock::Event::Unjoined do - agent.callback :disconnect, self, agent.call, queued_call + agent.callback :disconnect, queue, agent.call, queued_call ignoring_ended_calls { queued_call.hangup } ignoring_ended_calls { conditionally_return_agent agent if agent.call && agent.call.active? } agent.call[:queued_call] = nil if agent.call @@ -431,13 +434,13 @@ def bridge_agent(agent, queued_call) queued_call[:electric_slide_connected_at] = event.timestamp end - agent.callback :connect, self, agent.call, queued_call + 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? - agent.callback :connection_failed, self, agent.call, queued_call + agent.callback :connection_failed, current_actor, agent.call, queued_call logger.info "Caller #{queued_caller_id} failed to connect to Agent #{agent.id} due to caller hangup" conditionally_return_agent agent, :auto @@ -458,7 +461,7 @@ def bridged_agent_health_check(agent) abort ArgumentError.new("Agent has no active call") unless agent.call && agent.call.active? unless agent.call[:electric_slide_callback_set] agent.call[:electric_slide_callback_set] = true - queue = self + queue = current_actor agent.call.on_end do agent.call = nil queue.return_agent agent, :unavailable From fd2aa535acb3f1f93ec61ff9f1bf53a5d096813f Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Mon, 4 Jan 2016 20:24:07 -0200 Subject: [PATCH 2/2] Bump to 0.4.1 --- CHANGELOG.md | 2 ++ lib/electric_slide/version.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3a2d3d..456d48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ # [develop](https://github.com/adhearsion/electric_slide) + +# [0.4.1](https://github.com/adhearsion/electric_slide/compare/v0.4.0...v0.4.1) - [2016-01-04](https://rubygems.org/gems/adhearsion/versions/0.4.1) * Bugfix: Don't hand out internal CallQueue references to Agents - thread-safety # [0.4.0](https://github.com/adhearsion/electric_slide/compare/v0.3.0...v0.4.0) - [2015-12-17](https://rubygems.org/gems/adhearsion/versions/0.4.0) diff --git a/lib/electric_slide/version.rb b/lib/electric_slide/version.rb index 3e76a11..ab672bd 100644 --- a/lib/electric_slide/version.rb +++ b/lib/electric_slide/version.rb @@ -1,4 +1,4 @@ # encoding: utf-8 class ElectricSlide - VERSION = '0.4.0' + VERSION = '0.4.1' end