Merge lp:~phablet-team/media-hub/disconnect-playback_status_changed_signal into lp:media-hub

[Jim Hodapp]

Disconnect playback_status_changed_signal in ~Private() to avoid a deadlock.

* Disconnect playback_status_changed_signal in ~Private() to avoid a deadlock.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:131
http://jenkins.qa.ubuntu.com/job/media-hub-ci/320/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/160
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/160/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/160

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/320/rebuild

review: Approve (continuous-integration)

=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2015-03-19 19:11:01 +0000
+++ src/core/media/player_implementation.cpp	2015-03-19 19:11:01 +0000
@@ -62,8 +62,10 @@
           system_wakelock_count(0),
           display_wakelock_count(0),
           previous_state(Engine::State::stopped),
-          engine_state_change_connection(engine->state().changed().connect(make_state_change_handler()))
+          engine_state_change_connection(engine->state().changed().connect(make_state_change_handler())),
+          engine_playback_status_change_connection(engine->playback_status_changed_signal().connect(make_playback_status_change_handler()))
     {
+        std::cout << "Private parent instance: " << parent << std::endl;
         // Poor man's logging of release/acquire events.
         display_state_lock->acquired().connect([](media::power::DisplayState state)
         {
@@ -96,6 +98,12 @@
         // trigger the state change handler. Ensure the handler is not called
         // by disconnecting the state change signal
         engine_state_change_connection.disconnect();
+
+        std::cout << "** Disconnecting playback_status_changed_signal connection";
+        // The engine destructor can lead to a playback status change which will
+        // trigger the playback status change handler. Ensure the handler is not called
+        // by disconnecting the playback status change signal
+        engine_playback_status_change_connection.disconnect();
     }
 
     std::function<void(const Engine::State& state)> make_state_change_handler()
@@ -107,6 +115,7 @@
          */
         return [this](const Engine::State& state)
         {
+            std::cout << "Setting state for parent: " << parent << std::endl;
             switch(state)
             {
             case Engine::State::ready:
@@ -125,6 +134,7 @@
                 parent->meta_data_for_current_track().set(std::get<1>(engine->track_meta_data().get()));
                 // And update our playback status.
                 parent->playback_status().set(media::Player::playing);
+                std::cout << "Requesting power state" << std::endl;
                 request_power_state();
                 break;
             }
@@ -155,14 +165,25 @@
         };
     }
 
+    std::function<void(const media::Player::PlaybackStatus& status)> make_playback_status_change_handler()
+    {
+        return [this](const media::Player::PlaybackStatus& status)
+        {
+            std::cout << "Emiting playback_status_changed for parent: " << parent << std::endl;
+            parent->emit_playback_status_changed(status);
+        };
+    }
+
     void request_power_state()
     {
+        std::cout << __PRETTY_FUNCTION__ << std::endl;
         try
         {
             if (parent->is_video_source())
             {
                 if (++display_wakelock_count == 1)
                 {
+                    std::cout << "Requesting new display wakelock." << std::endl;
                     display_state_lock->request_acquire(media::power::DisplayState::on);
                     std::cout << "Requested new display wakelock." << std::endl;
                 }
@@ -171,6 +192,7 @@
             {
                 if (++system_wakelock_count == 1)
                 {
+                    std::cout << "Requesting new system wakelock." << std::endl;
                     system_state_lock->request_acquire(media::power::SystemState::active);
                     std::cout << "Requested new system wakelock." << std::endl;
                 }
@@ -274,6 +296,7 @@
     Engine::State previous_state;
     core::Signal<> on_client_disconnected;
     core::Connection engine_state_change_connection;
+    core::Connection engine_playback_status_change_connection;
 };
 
 template<typename Parent>
@@ -379,11 +402,6 @@
         Parent::end_of_stream()();
     });
 
-    d->engine->playback_status_changed_signal().connect([this](const Player::PlaybackStatus& status)
-    {
-        Parent::playback_status_changed()(status);
-    });
-
     d->engine->video_dimension_changed_signal().connect([this](const media::video::Dimensions& dimensions)
     {
         Parent::video_dimension_changed()(dimensions);
@@ -519,6 +537,12 @@
     return d->on_client_disconnected;
 }
 
+template<typename Parent>
+void media::PlayerImplementation<Parent>::emit_playback_status_changed(const media::Player::PlaybackStatus &status)
+{
+    Parent::playback_status_changed()(status);
+}
+
 #include <core/media/player_skeleton.h>
 
 // For linking purposes, we have to make sure that we have all symbols included within the dso.

=== modified file 'src/core/media/player_implementation.h'
--- src/core/media/player_implementation.h	2015-03-19 19:11:01 +0000
+++ src/core/media/player_implementation.h	2015-03-19 19:11:01 +0000
@@ -70,6 +70,10 @@
     virtual void seek_to(const std::chrono::microseconds& offset);
 
     const core::Signal<>& on_client_disconnected() const;
+
+protected:
+    void emit_playback_status_changed(const Player::PlaybackStatus &status);
+
 private:
     struct Private;
     std::shared_ptr<Private> d;