Merge 5e7b65d into c0a9e9b

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).

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
     #

lib/adhearsion/calls.rb

@@ -7,15 +7,15 @@ 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
 
     def <<(call)
       @supervisor.link call
       @mutex.synchronize do
-        self[call.id] = call
+        @calls[call.id] = call
         by_uri[call.uri] = call
       end
       self

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

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}"

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?

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
 

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
 

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

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