[BUGFIX] Ensure that hungup calls and merged dials can split/rejoin p…

…roperly

CHANGELOG.md

@@ -1,6 +1,8 @@
 # [develop](https://github.com/adhearsion/adhearsion)
   * Bugfix: Don't block shutdown waiting for the console to terminate
   * Bugfix: Ensure that splitting a dial rejoined to an alternative target (eg a mixer) or merged with another dial can still be split properly.
+  * Bugfix: Ensure that hungup calls don't prevent dial splits/merges.
+  * Bugfix: Ensure that merged dials which are split and rejoined all get rejoined to the mixer.
 
 # [2.4.0](https://github.com/adhearsion/adhearsion/compare/v2.3.5...v2.4.0) - [2013-08-29](https://rubygems.org/gems/adhearsion/versions/2.4.0)
   * Deprecation: Ruby 1.9.2 support is deprecated and will be dropped in a future version of Adhearsion

lib/adhearsion/call_controller/dial.rb

@@ -184,33 +184,39 @@ def split(targets = {})
           logger.info "Splitting calls apart"
           @splitting = true
           @calls.each do |call|
-            logger.info "Unjoining peer #{call.id} from #{join_target}"
-            ignoring_missing_joins { call.unjoin join_target }
-            if split_controller = targets[:others]
-              logger.info "Executing split controller #{split_controller} on #{call.id}"
-              call.execute_controller split_controller.new(call, 'current_dial' => self), targets[:others_callback]
+            ignoring_ended_calls do
+              logger.info "Unjoining peer #{call.id} from #{join_target}"
+              ignoring_missing_joins { call.unjoin join_target }
+              if split_controller = targets[:others]
+                logger.info "Executing split controller #{split_controller} on #{call.id}"
+                call.execute_controller split_controller.new(call, 'current_dial' => self), targets[:others_callback]
+              end
             end
           end
-          if join_target != @call
-            logger.info "Unjoining main call #{@call.id} from #{join_target}"
-            @call.unjoin join_target
-          end
-          if split_controller = targets[:main]
-            logger.info "Executing split controller #{split_controller} on main call"
-            @call.execute_controller split_controller.new(@call, 'current_dial' => self), targets[:main_callback]
+          ignoring_ended_calls do
+            if join_target != @call
+              logger.info "Unjoining main call #{@call.id} from #{join_target}"
+              @call.unjoin join_target
+            end
+            if split_controller = targets[:main]
+              logger.info "Executing split controller #{split_controller} on main call"
+              @call.execute_controller split_controller.new(@call, 'current_dial' => self), targets[:main_callback]
+            end
           end
         end
 
         # Rejoin parties that were previously split
         # @param [Call, String, Hash] target The target to join calls to. See Call#join for details.
-        def rejoin(target = @call)
+        def rejoin(target = join_target)
           logger.info "Rejoining to #{target}"
-          unless target == @call
-            @join_target = target
-            @call.join target
+          ignoring_ended_calls do
+            unless target == @call
+              @join_target = target
+              @call.join target
+            end
           end
           @calls.each do |call|
-            call.join target
+            ignoring_ended_calls { call.join target }
           end
         end
 
@@ -256,9 +262,8 @@ def skip_cleanup
         # Hangup any remaining calls
         def cleanup_calls
           calls_to_hangup = @calls.map do |call|
-            begin
+            ignoring_ended_calls do
               [call.id, call] if call.active?
-            rescue Celluloid::DeadActorError
             end
           end.compact
           if calls_to_hangup.size.zero?
@@ -270,11 +275,7 @@ def cleanup_calls
           else
             logger.info "#dial finished. Hanging up #{calls_to_hangup.size} outbound calls which are still active: #{calls_to_hangup.map(&:first).join ", "}."
             calls_to_hangup.each do |id, outbound_call|
-              begin
-                outbound_call.hangup
-              rescue Celluloid::DeadActorError
-                # This actor may previously have been shut down due to the call ending
-              end
+              ignoring_ended_calls { outbound_call.hangup }
             end
           end
         end
@@ -320,11 +321,9 @@ def pre_join_tasks(call)
 
         def on_all_except(call)
           @calls.each do |target_call|
-            begin
+            ignoring_ended_calls do
               next if target_call.id == call.id
               yield target_call
-            rescue Celluloid::DeadActorError
-              # This actor may previously have been shut down due to the call ending
             end
           end
         end
@@ -334,6 +333,12 @@ def ignoring_missing_joins
         rescue Punchblock::ProtocolError => e
           raise unless e.name == :service_unavailable
         end
+
+        def ignoring_ended_calls
+          yield
+        rescue Celluloid::DeadActorError, Adhearsion::Call::Hangup, Adhearsion::Call::ExpiredError
+          # This actor may previously have been shut down due to the call ending
+        end
       end
 
       class ParallelConfirmationDial < Dial

spec/adhearsion/call_controller/dial_spec.rb

@@ -600,6 +600,79 @@ def run
                 dial.status.result.should be == :answer
               end
 
+              it "should subsequently rejoin to a mixer" do
+                SecureRandom.stub uuid: 'foobar'
+                [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true }
+
+                dial.merge other_dial
+
+                waiter_thread = Thread.new do
+                  dial.await_completion
+                  latch.countdown!
+                end
+
+                sleep 0.5
+
+                other_mock_call << mock_end
+                latch.wait(1).should be_false
+
+                [call, other_mock_call, second_root_call, second_other_mock_call].each do |call|
+                  call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do
+                    call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar')
+                  end
+                end
+
+                dial.split
+
+                [call, other_mock_call, second_root_call, second_other_mock_call].each do |call|
+                  call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do
+                    call << Punchblock::Event::Joined.new(mixer_name: 'foobar')
+                  end
+                end
+
+                dial.rejoin
+              end
+
+              context "if a call hangs up" do
+                before { SecureRandom.stub uuid: 'foobar' }
+
+                it "should still allow splitting and rejoining" do
+                  [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true }
+
+                  dial.merge other_dial
+
+                  waiter_thread = Thread.new do
+                    dial.await_completion
+                    latch.countdown!
+                  end
+
+                  sleep 0.5
+
+                  other_mock_call << mock_end
+                  latch.wait(1).should be_false
+
+                  [call, second_root_call, second_other_mock_call].each do |call|
+                    call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do
+                      call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar')
+                    end
+                  end
+
+                  other_mock_call.should_receive(:unjoin).and_raise Adhearsion::Call::ExpiredError
+
+                  dial.split
+
+                  [call, second_root_call, second_other_mock_call].each do |call|
+                    call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do
+                      call << Punchblock::Event::Joined.new(mixer_name: 'foobar')
+                    end
+                  end
+
+                  other_mock_call.should_receive(:join).and_raise Adhearsion::Call::Hangup
+
+                  dial.rejoin
+                end
+              end
+
               context "if the calls were not joined" do
                 it "should still join to mixer" do
                   other_mock_call.should_receive(:unjoin).once.ordered.with(call).and_raise Punchblock::ProtocolError.new.setup(:service_unavailable)
@@ -1639,6 +1712,79 @@ def run
                 dial.status.result.should be == :answer
               end
 
+              it "should subsequently rejoin to a mixer" do
+                SecureRandom.stub uuid: 'foobar'
+                [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true }
+
+                dial.merge other_dial
+
+                waiter_thread = Thread.new do
+                  dial.await_completion
+                  latch.countdown!
+                end
+
+                sleep 0.5
+
+                other_mock_call << mock_end
+                latch.wait(1).should be_false
+
+                [call, other_mock_call, second_root_call, second_other_mock_call].each do |call|
+                  call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do
+                    call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar')
+                  end
+                end
+
+                dial.split
+
+                [call, other_mock_call, second_root_call, second_other_mock_call].each do |call|
+                  call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do
+                    call << Punchblock::Event::Joined.new(mixer_name: 'foobar')
+                  end
+                end
+
+                dial.rejoin
+              end
+
+              context "if a call hangs up" do
+                before { SecureRandom.stub uuid: 'foobar' }
+
+                it "should still allow splitting and rejoining" do
+                  [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true }
+
+                  dial.merge other_dial
+
+                  waiter_thread = Thread.new do
+                    dial.await_completion
+                    latch.countdown!
+                  end
+
+                  sleep 0.5
+
+                  other_mock_call << mock_end
+                  latch.wait(1).should be_false
+
+                  [call, second_root_call, second_other_mock_call].each do |call|
+                    call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do
+                      call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar')
+                    end
+                  end
+
+                  other_mock_call.should_receive(:unjoin).and_raise Adhearsion::Call::ExpiredError
+
+                  dial.split
+
+                  [call, second_root_call, second_other_mock_call].each do |call|
+                    call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do
+                      call << Punchblock::Event::Joined.new(mixer_name: 'foobar')
+                    end
+                  end
+
+                  other_mock_call.should_receive(:join).and_raise Adhearsion::Call::Hangup
+
+                  dial.rejoin
+                end
+              end
+
               context "if the calls were not joined" do
                 it "should still join to mixer" do
                   other_mock_call.should_receive(:unjoin).once.ordered.with(call).and_raise Punchblock::ProtocolError.new.setup(:service_unavailable)