diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d67b9d9..380c71b25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ # [develop](https://github.com/adhearsion/adhearsion) + * Feature: Increases concurrent call routing performance by delegating call routing to Call actors + * Bugfix: Reporting statistics should not deadlock call processing + * Bugfix: Shuts down a call which couldn't be routed with a fake end event. This avoids leaking call actors in cases where the call ended before it was dispatched and we missed the real End event. + * Bugfix: Removes proactive checks on call availability before dispatching events because these are inaccurate; now optimistically dispatches. # [2.6.2](https://github.com/adhearsion/adhearsion/compare/v2.6.1...v2.6.2) - [2015-06-16](https://rubygems.org/gems/adhearsion/versions/2.6.2) * Bugfix: Properly load application bundle. This requires the spawning removed in [#534](https://github.com/adhearsion/adhearsion/pull/534). diff --git a/lib/adhearsion/call.rb b/lib/adhearsion/call.rb index 4bbe2d8b5..05e68a912 100644 --- a/lib/adhearsion/call.rb +++ b/lib/adhearsion/call.rb @@ -314,7 +314,7 @@ def answer(headers = nil) def reject(reason = :busy, headers = nil) write_and_await_response Punchblock::Command::Reject.new(:reason => reason, :headers => headers) - Adhearsion::Events.trigger_immediately :call_rejected, call: current_actor, reason: reason + Adhearsion::Events.trigger :call_rejected, call: current_actor, reason: reason rescue Punchblock::ProtocolError => e abort e end @@ -491,6 +491,22 @@ def write_command(command) client.execute_command command end + def route + case Adhearsion::Process.state_name + when :booting, :rejecting + logger.info "Declining call because the process is not yet running." + reject :decline + when :running, :stopping + logger.info "Routing call" + Adhearsion.router.handle current_actor + else + reject :error + end + rescue Call::Hangup, Call::ExpiredError + logger.warn "Call routing could not be completed because call was unavailable." + self << Punchblock::Event::End.new(reason: :error) + end + ## # Sends a message to the caller # diff --git a/lib/adhearsion/calls.rb b/lib/adhearsion/calls.rb index 349b13ff7..b437ab3c7 100644 --- a/lib/adhearsion/calls.rb +++ b/lib/adhearsion/calls.rb @@ -7,7 +7,7 @@ module Adhearsion # This manages the list of calls the Adhearsion service receives class Calls def initialize - @mutex = ::Monitor.new + @mutex = ::Mutex.new @calls = {} restart_supervisor end @@ -15,7 +15,7 @@ def initialize def <<(call) @supervisor.link call @mutex.synchronize do - self[call.id] = call + @calls[call.id] = call by_uri[call.uri] = call end self diff --git a/lib/adhearsion/outbound_call.rb b/lib/adhearsion/outbound_call.rb index 89d1d7e08..0417eb993 100644 --- a/lib/adhearsion/outbound_call.rb +++ b/lib/adhearsion/outbound_call.rb @@ -99,7 +99,7 @@ def dial(to, options = {}) Adhearsion.active_calls << current_actor Adhearsion.active_calls.delete(@id) end - Adhearsion::Events.trigger_immediately :call_dialed, current_actor + Adhearsion::Events.trigger :call_dialed, current_actor end rescue clear_from_active_calls diff --git a/lib/adhearsion/punchblock_plugin/initializer.rb b/lib/adhearsion/punchblock_plugin/initializer.rb index 0de185c7a..97695859a 100644 --- a/lib/adhearsion/punchblock_plugin/initializer.rb +++ b/lib/adhearsion/punchblock_plugin/initializer.rb @@ -130,21 +130,12 @@ def dispatch_offer(offer) catching_standard_errors do call = Call.new(offer) Adhearsion.active_calls << call - case Adhearsion::Process.state_name - when :booting, :rejecting - logger.info "Declining call because the process is not yet running." - call.reject :decline - when :running, :stopping - Adhearsion.router.handle call - else - call.reject :error - end + call.async.route end end def dispatch_call_event(event) - call = Adhearsion.active_calls[event.target_call_id] - if call && call.alive? + if call = Adhearsion.active_calls[event.target_call_id] call.async.deliver_message event else logger.warn "Event received for inactive call #{event.target_call_id}: #{event.inspect}" diff --git a/lib/adhearsion/router/route.rb b/lib/adhearsion/router/route.rb index 1e85b327f..ce00af9cf 100644 --- a/lib/adhearsion/router/route.rb +++ b/lib/adhearsion/router/route.rb @@ -26,7 +26,7 @@ def match?(call) end def dispatch(call, callback = nil) - Adhearsion::Events.trigger_immediately :call_routed, call: call, route: self + Adhearsion::Events.trigger :call_routed, call: call, route: self call_id = call.id # Grab this to use later incase the actor is dead @@ -54,8 +54,6 @@ def dispatch(call, callback = nil) end callback.call if callback } - rescue Call::Hangup, Call::ExpiredError - logger.info "Call routing could not be completed because call was unavailable." end def evented? diff --git a/spec/adhearsion/call_spec.rb b/spec/adhearsion/call_spec.rb index d861ef49d..25e4a2631 100644 --- a/spec/adhearsion/call_spec.rb +++ b/spec/adhearsion/call_spec.rb @@ -39,6 +39,16 @@ module Adhearsion Adhearsion.active_calls.clear end + def expect_message_waiting_for_response(message = nil, fail = false, &block) + expectation = expect(subject.wrapped_object).to receive(:write_and_await_response, &block).once + expectation = expectation.with message if message + if fail + expectation.and_raise fail + else + expectation.and_return message + end + end + it "should do recursion detection on inspect" do subject[:foo] = subject Timeout.timeout(0.2) do @@ -855,6 +865,51 @@ module Adhearsion end end + describe "routing" do + before do + expect(Adhearsion::Process).to receive(:state_name).once.and_return process_state + end + + after { subject.route } + + context "when the Adhearsion::Process is :booting" do + let(:process_state) { :booting } + + it 'should reject a call with cause :declined' do + expect_message_waiting_for_response Punchblock::Command::Reject.new(reason: :decline) + end + end + + [ :running, :stopping ].each do |state| + context "when when Adhearsion::Process is in :#{state}" do + let(:process_state) { state } + + it "should dispatch via the router" do + Adhearsion.router do + route 'foobar', Class.new + end + expect(Adhearsion.router).to receive(:handle).once.with subject + end + end + end + + context "when when Adhearsion::Process is in :rejecting" do + let(:process_state) { :rejecting } + + it 'should reject a call with cause :declined' do + expect_message_waiting_for_response Punchblock::Command::Reject.new(reason: :decline) + end + end + + context "when when Adhearsion::Process is not :running, :stopping or :rejecting" do + let(:process_state) { :foobar } + + it 'should reject a call with cause :error' do + expect_message_waiting_for_response Punchblock::Command::Reject.new(reason: :error) + end + end + end + describe "#send_message" do it "should send a message through the Punchblock connection using the call ID and domain" do expect(subject.wrapped_object).to receive(:client).once.and_return mock_client @@ -870,16 +925,6 @@ module Adhearsion end describe "basic control commands" do - def expect_message_waiting_for_response(message = nil, fail = false, &block) - expectation = expect(subject.wrapped_object).to receive(:write_and_await_response, &block).once - expectation = expectation.with message if message - if fail - expectation.and_raise fail - else - expectation.and_return message - end - end - describe '#accept' do describe "with no headers" do it 'should send an Accept message' do @@ -978,7 +1023,7 @@ def expect_message_waiting_for_response(message = nil, fail = false, &block) it "should immediately fire the :call_rejected event giving the call and the reason" do expect_message_waiting_for_response kind_of(Punchblock::Command::Reject) - expect(Adhearsion::Events).to receive(:trigger_immediately).once.with(:call_rejected, :call => subject, :reason => :decline) + expect(Adhearsion::Events).to receive(:trigger).once.with(:call_rejected, :call => subject, :reason => :decline) subject.reject :decline end diff --git a/spec/adhearsion/outbound_call_spec.rb b/spec/adhearsion/outbound_call_spec.rb index 3b7c730ab..e45df1e9d 100644 --- a/spec/adhearsion/outbound_call_spec.rb +++ b/spec/adhearsion/outbound_call_spec.rb @@ -280,7 +280,7 @@ def expect_message_waiting_for_response(message, uri = call_uri) end it "should immediately fire the :call_dialed event giving the call" do - expect(Adhearsion::Events).to receive(:trigger_immediately).once.with(:call_dialed, subject) + expect(Adhearsion::Events).to receive(:trigger).once.with(:call_dialed, subject) subject.dial to, :from => from end diff --git a/spec/adhearsion/punchblock_plugin/initializer_spec.rb b/spec/adhearsion/punchblock_plugin/initializer_spec.rb index 1d370362c..4f9634d78 100644 --- a/spec/adhearsion/punchblock_plugin/initializer_spec.rb +++ b/spec/adhearsion/punchblock_plugin/initializer_spec.rb @@ -41,7 +41,7 @@ def initialize_punchblock(options = {}) Adhearsion.config[:punchblock] end - let(:call_id) { rand } + let(:call_id) { rand.to_s } let(:offer) { Punchblock::Event::Offer.new :target_call_id => call_id } let(:mock_call) { Call.new } let(:mock_client) { double 'Client' } @@ -198,48 +198,13 @@ def initialize_punchblock(options = {}) describe "dispatching an offer" do before do initialize_punchblock - expect(Adhearsion::Process).to receive(:state_name).once.and_return process_state - expect(Adhearsion::Call).to receive(:new).once.and_return mock_call - end - - context "when the Adhearsion::Process is :booting" do - let(:process_state) { :booting } - - it 'should reject a call with cause :declined' do - expect(mock_call).to receive(:reject).once.with(:decline) - end - end - - [ :running, :stopping ].each do |state| - context "when when Adhearsion::Process is in :#{state}" do - let(:process_state) { state } - - it "should dispatch via the router" do - Adhearsion.router do - route 'foobar', Class.new - end - expect(Adhearsion.router).to receive(:handle).once.with mock_call - end - end - end - - context "when when Adhearsion::Process is in :rejecting" do - let(:process_state) { :rejecting } - - it 'should reject a call with cause :declined' do - expect(mock_call).to receive(:reject).once.with(:decline) - end end - context "when when Adhearsion::Process is not :running, :stopping or :rejecting" do - let(:process_state) { :foobar } - - it 'should reject a call with cause :error' do - expect(mock_call).to receive(:reject).once.with(:error) - end + it "should create and route the call" do + expect(Adhearsion::Call).to receive(:new).once.and_return mock_call + expect(mock_call).to receive_message_chain("async.route") + Initializer.dispatch_offer offer end - - after { Events.trigger_immediately :punchblock, offer } end describe "dispatching a component event" do @@ -280,7 +245,6 @@ def initialize_punchblock(options = {}) end it "should not block on the call handling the event" do - mock_call.register_event_handler do |event| sleep 5 end @@ -297,18 +261,6 @@ def initialize_punchblock(options = {}) Initializer.dispatch_call_event mock_event end end - - describe "when the registry contains a dead call" do - before do - mock_call.terminate - Adhearsion.active_calls[mock_call.id] = mock_call - end - - it "should log a warning" do - expect(Adhearsion::Logging.get_logger(Initializer)).to receive(:warn).once.with("Event received for inactive call #{call_id}: #{mock_event.inspect}") - Initializer.dispatch_call_event mock_event - end - end end context "Punchblock configuration" do diff --git a/spec/adhearsion/router/route_spec.rb b/spec/adhearsion/router/route_spec.rb index f9794599b..6e8c82d62 100644 --- a/spec/adhearsion/router/route_spec.rb +++ b/spec/adhearsion/router/route_spec.rb @@ -138,7 +138,7 @@ def should_not_match_the_call let(:route) { Route.new 'foobar', controller } it "should immediately fire the :call_routed event giving the call and route" do - expect(Adhearsion::Events).to receive(:trigger_immediately).once.with(:call_routed, call: call, route: route) + expect(Adhearsion::Events).to receive(:trigger).once.with(:call_routed, call: call, route: route) expect(call).to receive(:hangup).once route.dispatch call, lambda { latch.countdown! } expect(latch.wait(2)).to be true @@ -164,16 +164,8 @@ def should_not_match_the_call context "when the call has already ended before routing can begin" do before { Celluloid::Actor.kill call } - it "should fall through cleanly" do - expect { route.dispatch call }.not_to raise_error - end - end - - context "when the call has already ended before routing can begin" do - before { Celluloid::Actor.kill call } - - it "should fall through cleanly" do - expect { route.dispatch call }.not_to raise_error + it "should raise a hangup exception" do + expect { route.dispatch call }.to raise_error(Call::ExpiredError) end end