Merge lp:~unity-api-team/indicator-network/fix-gmainloop-syncing into lp:indicator-network/14.09

[Antti Kaijanmäki]

* Fix GMainLoopDispatch dispatching ordering when
  called inside GMainLoop already
* Fix GMainLoopDispatch locking
* Force all signals from external (dbus-cpp) threads through
  GMainLoopDispatch

[Charles Kerr (charlesk)]

I have mixed feelings about this patch.

The "big picture" is this is a step in the right direction and my vote is to approve. I don't like the technical debt being accrued here, but it's preferable to blocking Criticals on an even larger megapatch.

First, The "shared pimpl" idiom of thread safety here is interesting. It's generic enough that it can be (and is) applied liberally here, so that's useful. TBH though, it doesn't make sense to keep processing these signals after the outer object's been destroyed and I would have preferred another solution, e.g. GMainLoopDispatch's destructor calling g_signal_remove() after checking a lock guard to confirm that the func wasn't being called at the same time.

Second, the new comments sprinkled through the code "this must be called from GMainLoop", "these signals are already synced in main loop" etc. are red flags. There are a lot of them and they're hard to test... if there's no code making incorrect calls now (and how would we know for certian?) there surely will be in the future. I'd rather see either (a) a compile-time test of some kind, although I don't know how that would work, or (b) consistent thread rules applied through the code so that everyone knows what the rules of the road are.

Third, many of the ScopedConnections created by connect() are dropped on the floor. Since these connections' lambdas hold handles to the pimpl shared_ptrs, we need to be more careful about disconnecting them when the public objects are destroyed. My concern is that we don't wind up with ghost object chains that continue to process each other's signals.

Lastly (and superficially), I'd also like to see GMainLoopDispatch renamed MainLoopDispatch, since there's nothing intrinsic to glib about the idea. :)

review: Approve

[Antti Kaijanmäki (kaijanmaki)]

> The "big picture" is this is a step in the right direction and my vote is to
> approve. I don't like the technical debt being accrued here, but it's
> preferable to blocking Criticals on an even larger megapatch.

The MP tries to do minimal changes, but yet introduce the dispatching of all external signals through GMainLoopDispatch.

> First, The "shared pimpl" idiom of thread safety here is interesting. It's
> generic enough that it can be (and is) applied liberally here, so that's
> useful. TBH though, it doesn't make sense to keep processing these signals
> after the outer object's been destroyed and I would have preferred another
> solution, e.g. GMainLoopDispatch's destructor calling g_signal_remove() after
> checking a lock guard to confirm that the func wasn't being called at the same
> time.

please note that GMainLoopDispatch is not a long living object. It's destructor is not usable for virtually anything.

GMainLoopDispatch is destroyed as soon as it gets out of scope in which it was created. It only exists to offer easy interface to the static variables inside menumodel-cpp/gio-helpers/utils.cpp.

> Second, the new comments sprinkled through the code "this must be called from
> GMainLoop", "these signals are already synced in main loop" etc. are red
> flags. There are a lot of them and they're hard to test... if there's no code
> making incorrect calls now (and how would we know for certian?) there surely
> will be in the future. I'd rather see either (a) a compile-time test of some
> kind, although I don't know how that would work, or (b) consistent thread
> rules applied through the code so that everyone knows what the rules of the
> road are.

Yes. the problem here is that the code base did not rely on GMainLoop being around at all. This is the first step to more widely utilize GMainLoop. It's already agreed that we will create certain assertion directive that checks it is being called from GMainLoop main context. This directive can then be slapped through the codebase to verify that the precondition holds.

> Third, many of the ScopedConnections created by connect() are dropped on the
> floor. Since these connections' lambdas hold handles to the pimpl shared_ptrs,

The only lambdas holding pimpl shared_ptrs are the ones passed to GMainLoopDispatch and the lambdas are free'ed as soon as the dispatching is done.

> we need to be more careful about disconnecting them when the public objects
> are destroyed. My concern is that we don't wind up with ghost object chains
> that continue to process each other's signals.

We share the same concern. Next step on robusting the code base is to indeed to vefiry the lifecycles. Any connections that there are now in place or are missing are coming from the code base prior of this MP. And verifying and fixing possible issues is left for next iteration to keep the MP size and mental burden down.

> Lastly (and superficially), I'd also like to see GMainLoopDispatch renamed
> MainLoopDispatch, since there's nothing intrinsic to glib about the idea. :)

here we go again... ;)

[Marcus Tomlinson (marcustomlinson)]

Tested this on mako rtm 115, and it does fix the linked bug. Next we should scan through the rest of the code for callbacks that could arrive on external threads (E.g. service side writable properties whose 'changed' signals are emitted from the dbus-cpp thread)

review: Approve

[lp:~unity-api-team/indicator-network/fix-gmainloop-syncing]

468. By Antti Kaijanmäki on 2014-10-31

races races all the way..

469. By Antti Kaijanmäki on 2014-10-31

blah..

=== modified file 'src/connectivity-cpp/dbus-cpp/services/nm.h'
--- src/connectivity-cpp/dbus-cpp/services/nm.h	2014-10-31 14:07:40 +0000
+++ src/connectivity-cpp/dbus-cpp/services/nm.h	2014-10-31 14:07:40 +0000
@@ -911,7 +911,7 @@
                 return result.value();
             }
 
-            std::vector<Device> get_devices()
+            std::vector<core::dbus::types::ObjectPath> get_devices()
             {
                 auto result =
                         object->invoke_method_synchronously<
@@ -921,16 +921,7 @@
                 if (result.is_error())
                     connectivity::throw_dbus_exception(result.error());
 
-                std::vector<Device> devices;
-                for (const auto& path : result.value())
-                {
-                    devices.emplace_back(
-                                Device(
-                                    service,
-                                    service->object_for_path(path)));
-                }
-
-                return devices;
+                return result.value();
             }
 
             NetworkManager(std::shared_ptr<core::dbus::Service> &service,

=== modified file 'src/connectivity-cpp/src/platform/nmofono/manager.cpp'
--- src/connectivity-cpp/src/platform/nmofono/manager.cpp	2014-10-09 10:58:30 +0000
+++ src/connectivity-cpp/src/platform/nmofono/manager.cpp	2014-10-31 14:07:40 +0000
@@ -172,29 +172,11 @@
                                                        p->urfkill->switches[fdo::URfkill::Interface::Killswitch::Type::wlan]);
     p->m_wifiKillSwitch->state().changed().connect(std::bind(&Private::updateHasWifi, p.get()));
 
-    p->nm->device_added->connect([this](const dbus::types::ObjectPath &path){
-#ifdef INDICATOR_NETWORK_TRACE_MESSAGES
-        std::cout << "Device Added:" << path.as_string() << std::endl;
-#endif
-        auto links = p->m_links.get();
-        for (const auto &dev : links) {
-            if (std::dynamic_pointer_cast<wifi::Link>(dev)->device_path() == path) {
-                // already in the list
-                return;
-            }
-        }
-
-        NM::Interface::Device dev(p->nm->service,
-                                  p->nm->service->object_for_path(path));
-        if (dev.type() == NM::Interface::Device::Type::wifi) {
-            links.insert(std::make_shared<wifi::Link>(dev,
-                                                      *p->nm.get(),
-                                                       p->m_wifiKillSwitch));
-            p->m_links.set(links);
-        }
-
-        p->updateHasWifi();
-    });
+    p->nm->device_added->connect(std::bind(&Manager::device_added, this, std::placeholders::_1));
+    for(const auto &path : p->nm->get_devices()) {
+        device_added(path);
+    }
+
     p->nm->device_removed->connect([this](const dbus::types::ObjectPath &path){
 #ifdef INDICATOR_NETWORK_TRACE_MESSAGES
         std::cout << "Device Removed:" << path.as_string() << std::endl;
@@ -210,23 +192,6 @@
 
         p->updateHasWifi();
     });
-    std::set<std::shared_ptr<networking::Link> > links;
-    for(auto dev : p->nm->get_devices()) {
-        switch (dev.type()) {
-        case NM::Interface::Device::Type::wifi:
-        {
-            std::shared_ptr<networking::Link> link;
-            link.reset(new wifi::Link(dev,
-                                      *p->nm.get(),
-                                      p->m_wifiKillSwitch));
-            links.insert(link);
-            break;
-        }
-        default:
-            ;
-        }
-    }
-    p->m_links.set(links);
 
     updateNetworkingStatus(p->nm->state->get());
     p->nm->properties_changed->connect([this](NM::Interface::NetworkManager::Signal::PropertiesChanged::ArgumentType map) {
@@ -259,6 +224,46 @@
 }
 
 void
+Manager::device_added(const dbus::types::ObjectPath &path)
+{
+#ifdef INDICATOR_NETWORK_TRACE_MESSAGES
+    std::cout << "Device Added:" << path.as_string() << std::endl;
+#endif
+    auto links = p->m_links.get();
+
+    for (const auto &dev : links) {
+        if (std::dynamic_pointer_cast<wifi::Link>(dev)->device_path() == path) {
+            // already in the list
+            return;
+        }
+    }
+
+    connectivity::networking::Link::Ptr link;
+    try {
+        NM::Interface::Device dev(p->nm->service,
+                                  p->nm->service->object_for_path(path));
+        if (dev.type() == NM::Interface::Device::Type::wifi) {
+            link = std::make_shared<wifi::Link>(dev,
+                                                *p->nm.get(),
+                                                p->m_wifiKillSwitch);
+        }
+    } catch (const std::exception &e) {
+        std::cerr << __PRETTY_FUNCTION__ << ": failed to create Device proxy for "<< path.as_string() << ": " << std::endl
+                  << "\t" << e.what() << std::endl
+                  << "\tIgnoring." << std::endl;
+        return;
+    }
+
+    if (link) {
+        links.insert(link);
+        p->m_links.set(links);
+    }
+
+    p->updateHasWifi();
+}
+
+
+void
 Manager::enableFlightMode()
 {
 #ifdef INDICATOR_NETWORK_TRACE_MESSAGES

=== modified file 'src/connectivity-cpp/src/platform/nmofono/manager.h'
--- src/connectivity-cpp/src/platform/nmofono/manager.h	2014-09-25 00:06:12 +0000
+++ src/connectivity-cpp/src/platform/nmofono/manager.h	2014-10-31 14:07:40 +0000
@@ -20,6 +20,7 @@
 #define PLATFORM_NMOFONO_MANAGER
 
 #include <connectivity/networking/manager.h>
+#include <core/dbus/types/object_path.h>
 
 namespace core {
 namespace dbus {
@@ -61,6 +62,9 @@
     const core::Property<std::set<std::shared_ptr<connectivity::networking::Service>>>&services() const override;
     const core::Property<connectivity::networking::Manager::NetworkingStatus> & status() const override;
     const core::Property<std::uint32_t>& characteristics() const override;
+
+private:
+    void device_added(const core::dbus::types::ObjectPath &path);
 };
 
 #endif

=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/access-point.cpp'
--- src/connectivity-cpp/src/platform/nmofono/wifi/access-point.cpp	2014-08-22 12:18:55 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/access-point.cpp	2014-10-31 14:07:40 +0000
@@ -34,11 +34,11 @@
 
     std::string ssid;
     // Note: raw_ssid is _not_ guaranteed to be null terminated.
-    const std::vector<std::int8_t> &raw_ssid = m_ap.ssid->get();
-    if(g_utf8_validate((const char*)(&raw_ssid[0]), raw_ssid.size(), nullptr)) {
-        ssid = std::string(raw_ssid.begin(), raw_ssid.end());
+    m_raw_ssid = m_ap.ssid->get();
+    if(g_utf8_validate((const char*)(&m_raw_ssid[0]), m_raw_ssid.size(), nullptr)) {
+        ssid = std::string(m_raw_ssid.begin(), m_raw_ssid.end());
     } else {
-        for (auto c : m_ap.ssid->get()) {
+        for (auto c : m_raw_ssid) {
             if (isprint(c)) {
                 ssid += (char)c;
             } else {
@@ -58,6 +58,15 @@
             }
         }
     });
+
+    m_flags = m_ap.flags->get();
+    /* NetworkManager seems to set the wpa and rns flags
+     * for AccessPoints on the same network in a total random manner.
+     * Sometimes only wpa_flags or rns_flags is set and sometimes
+     * they both are set but always to the same value
+     */
+    m_secflags = m_ap.wpa_flags->get()|m_ap.rsn_flags->get();
+    m_mode = m_ap.mode->get();
 }
 
 const core::dbus::types::ObjectPath AccessPoint::object_path() const {
@@ -79,6 +88,11 @@
     return m_ssid;
 }
 
+const std::vector<std::int8_t>& AccessPoint::raw_ssid() const
+{
+    return m_raw_ssid;
+}
+
 bool AccessPoint::secured() const
 {
     return m_secured;
@@ -93,14 +107,9 @@
     if(this == &other)
         return true;
     return m_ssid == other.m_ssid &&
-            m_ap.flags->get() == other.m_ap.flags->get() &&
-            /* NetworkManager seems to set the wpa and rns flags
-             * for AccessPoints on the same network in a total random manner.
-             * Sometimes only wpa_flags or rns_flags is set and sometimes
-             * they both are set but always to the same value
-             */
-            (m_ap.wpa_flags->get()|m_ap.rsn_flags->get()) == (other.m_ap.wpa_flags->get()|other.m_ap.rsn_flags->get()) &&
-            m_ap.mode->get() == other.m_ap.mode->get();
+            m_flags == other.m_flags &&
+            m_secflags == other.m_secflags &&
+            m_mode == other.m_mode;
 }
 
 }

=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/access-point.h'
--- src/connectivity-cpp/src/platform/nmofono/wifi/access-point.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/access-point.h	2014-10-31 14:07:40 +0000
@@ -40,10 +40,52 @@
 
 class AccessPoint : public connectivity::networking::wifi::AccessPoint
 {
-
 public:
     typedef std::shared_ptr<AccessPoint> Ptr;
 
+    struct Key {
+        std::string ssid;
+        uint32_t flags;
+        uint32_t secflags;
+        uint32_t mode;
+
+        bool operator<(const Key &other) const
+        {
+            // Standard lexigraphic comparison.
+            if(ssid < other.ssid)
+                return true;
+            if(ssid > other.ssid)
+                return false;
+
+            if(flags < other.flags)
+                return true;
+            if(flags > other.flags)
+                return false;
+
+            if(secflags  < other.secflags)
+                return true;
+            if(secflags > secflags)
+                return false;
+
+            if(mode < other.mode)
+                return true;
+            if(mode > other.mode)
+                return false;
+            return false;
+        }
+
+        Key() = delete;
+        Key(const AccessPoint::Ptr &curap)
+        {
+            ssid = curap->ssid();
+            flags = curap->m_flags;
+            secflags = curap->m_secflags;
+            mode = curap->m_mode;
+        }
+    };
+    friend class Key;
+
+
     AccessPoint(const org::freedesktop::NetworkManager::Interface::AccessPoint &ap);
     const core::Property<double>& strength() const;
     virtual ~AccessPoint() = default;
@@ -54,6 +96,7 @@
     const core::Property<std::chrono::system_clock::time_point>& lastConnected() const;
 
     const std::string& ssid() const override;
+    const std::vector<std::int8_t>& raw_ssid() const;
 
     bool secured() const override;
 
@@ -61,8 +104,6 @@
 
     const core::dbus::types::ObjectPath object_path() const;
 
-    const org::freedesktop::NetworkManager::Interface::AccessPoint& get_ap() const { return m_ap; }
-
     bool operator==(const platform::nmofono::wifi::AccessPoint &other) const;
     bool operator!=(const platform::nmofono::wifi::AccessPoint &other) const { return !(*this == other); };
 
@@ -71,8 +112,13 @@
     core::Property<std::chrono::system_clock::time_point> m_lastConnected;
     org::freedesktop::NetworkManager::Interface::AccessPoint m_ap;
     std::string m_ssid;
+    std::vector<std::int8_t> m_raw_ssid;
     bool m_secured;
     bool m_adhoc;
+
+    std::uint32_t m_flags;
+    std::uint32_t m_secflags;
+    std::uint32_t m_mode;
 };
 
 }

=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.cpp'
--- src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.cpp	2014-10-09 10:58:30 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.cpp	2014-10-31 14:07:40 +0000
@@ -156,11 +156,6 @@
     return p->aplist.at(0)->object_path();
 }
 
-const org::freedesktop::NetworkManager::Interface::AccessPoint& GroupedAccessPoint::get_ap() const {
-    std::lock_guard<std::mutex> l(p->m);
-    return p->aplist.at(0)->get_ap();
-}
-
 const core::Property<double>& GroupedAccessPoint::strength() const
 {
     std::lock_guard<std::mutex> l(p->m);
@@ -177,7 +172,12 @@
 {
     std::lock_guard<std::mutex> l(p->m);
     return p->aplist.at(0)->ssid();
+}
 
+const std::vector<std::int8_t>& GroupedAccessPoint::raw_ssid() const
+{
+    std::lock_guard<std::mutex> l(p->m);
+    return p->aplist.at(0)->raw_ssid();
 }
 
 bool GroupedAccessPoint::secured() const

=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.h'
--- src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/grouped-access-point.h	2014-10-31 14:07:40 +0000
@@ -57,6 +57,7 @@
     const core::Property<std::chrono::system_clock::time_point>& lastConnected() const;
 
     const std::string& ssid() const override;
+    const std::vector<std::int8_t>& raw_ssid() const;
 
     bool secured() const override;
 
@@ -64,8 +65,6 @@
 
     const core::dbus::types::ObjectPath object_path() const;
 
-    const org::freedesktop::NetworkManager::Interface::AccessPoint& get_ap() const;
-
     void add_ap(std::shared_ptr<platform::nmofono::wifi::AccessPoint> &ap);
     void remove_ap(std::shared_ptr<platform::nmofono::wifi::AccessPoint> &ap);
     int num_aps() const;

=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp'
--- src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp	2014-10-09 10:58:30 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp	2014-10-31 14:07:40 +0000
@@ -30,58 +30,6 @@
 
 using core::Property;
 
-struct AccessPointKey {
-    std::string ssid;
-    uint32_t flags;
-    uint32_t wpa_flags;
-    uint32_t rsn_flags;
-    uint32_t mode;
-
-    bool operator<(const AccessPointKey &other) const
-    {
-        // Standard lexigraphic comparison.
-        if(ssid < other.ssid)
-            return true;
-        if(ssid > other.ssid)
-            return false;
-
-        if(flags < other.flags)
-            return true;
-        if(flags > other.flags)
-            return false;
-
-        /* NetworkManager seems to set the wpa and rns flags
-         * for AccessPoints on the same network in a total random manner.
-         * Sometimes only wpa_flags or rns_flags is set and sometimes
-         * they both are set but always to the same value
-         */
-        if((wpa_flags|rsn_flags)  < (other.wpa_flags|other.rsn_flags))
-            return true;
-        if((wpa_flags|rsn_flags) > (other.wpa_flags|other.rsn_flags))
-            return false;
-
-        if(mode < other.mode)
-            return true;
-        if(mode > other.mode)
-            return false;
-        return false;
-    }
-};
-
-namespace {
-
-AccessPointKey build_key(const std::shared_ptr<platform::nmofono::wifi::AccessPoint> &curap)
-{
-    AccessPointKey k;
-    k.ssid = curap->ssid();
-    k.flags = curap->get_ap().flags->get();
-    k.wpa_flags = curap->get_ap().wpa_flags->get();
-    k.rsn_flags = curap->get_ap().rsn_flags->get();
-    k.mode = curap->get_ap().mode->get();
-    return k;
-}
-
-}
 
 struct Link::Private
 {
@@ -103,7 +51,7 @@
     // hack hack
     std::vector<core::ScopedConnection> switchConnection;
 
-    std::map<AccessPointKey, std::shared_ptr<GroupedAccessPoint>> grouper;
+    std::map<AccessPoint::Key, std::shared_ptr<GroupedAccessPoint>> grouper;
     bool disabled;
     std::uint32_t lastState;
     std::string name;
@@ -165,13 +113,23 @@
                 return;
             }
         }
+
+        platform::nmofono::wifi::AccessPoint::Ptr shap;
+        try {
+            NM::Interface::AccessPoint ap(p->nm.service->object_for_path(path));
+            shap = std::make_shared<platform::nmofono::wifi::AccessPoint>(ap);
+        } catch(const std::exception &e) {
+            std::cerr << __PRETTY_FUNCTION__ << ": failed to create AccessPoint proxy for "<< path.as_string() << ": " << std::endl
+                      << "\t" << e.what() << std::endl
+                      << "\tIgnoring." << std::endl;
+            return;
+        }
+
         auto list = p->rawAccessPoints.get();
-        NM::Interface::AccessPoint ap(p->nm.service->object_for_path(path));
-        auto shap = std::make_shared<platform::nmofono::wifi::AccessPoint>(ap);
         list.insert(shap);
         p->rawAccessPoints.set(list);
 
-        auto k = build_key(shap);
+        auto k = AccessPoint::Key(shap);
         if(p->grouper.find(k) != p->grouper.end()) {
             p->grouper[k]->add_ap(shap);
         } else {
@@ -200,7 +158,7 @@
         }
     }
     if (!shap) {
-        std::cerr << "Tried to remove access point " << path.as_string() << " that has not been added." << std::endl;
+        std::cerr << __PRETTY_FUNCTION__ << ": Tried to remove access point " << path.as_string() << " that has not been added." << std::endl;
         return;
     }
     p->rawAccessPoints.set(list);
@@ -345,11 +303,11 @@
         // The accesspoint interface does not provide this property so we need to coax it out of
         // derived classes.
         if(ap) {
-            ssid = ap->get_ap().ssid->get();
+            ssid = ap->raw_ssid();
         } else {
             std::shared_ptr<AccessPoint> bap = std::dynamic_pointer_cast<AccessPoint>(accessPoint);
             assert(bap);
-            ssid = bap->get_ap().ssid->get();
+            ssid = bap->raw_ssid();
         }
         NM::Interface::Connection *found = nullptr;
         auto connections = p->dev.get_available_connections();
@@ -360,7 +318,7 @@
                         if (conf.first == "ssid") {
                             std::vector<int8_t> value;
                             value = conf.second.as<std::vector<std::int8_t>>();
-                            if (value == ap->get_ap().ssid->get()) {
+                            if (value == ap->raw_ssid()) {
                                 found = &con;
                                 break;
                             }
@@ -384,7 +342,7 @@
             /// @todo getting the ssid multiple times over dbus is stupid.
 
             std::map<std::string, dbus::types::Variant> wireless_conf;
-            wireless_conf["ssid"] = dbus::types::Variant::encode<std::vector<std::int8_t>>(ap->get_ap().ssid->get());
+            wireless_conf["ssid"] = dbus::types::Variant::encode<std::vector<std::int8_t>>(ap->raw_ssid());
 
             conf["802-11-wireless"] = wireless_conf;
             auto ret = p->nm.add_and_activate_connection(conf,

=== modified file 'src/indicator/CMakeLists.txt'
--- src/indicator/CMakeLists.txt	2014-08-19 21:03:04 +0000
+++ src/indicator/CMakeLists.txt	2014-10-31 14:07:40 +0000
@@ -7,19 +7,18 @@
     service.h
     indicator-menu.h
     modem.cpp
-    modem.h
     modem-manager.cpp
     quick-access-section.cpp
     sim-unlock-dialog.cpp
     root-state.cpp
-    wifi-link-item.h
+    wifi-link-item.cpp
     wifi-section.cpp
     wwan-link-item.cpp
     wwan-section.cpp
 
     connectivity-service/connectivity-service.cpp
 
-    menuitems/access-point-item.h
+    menuitems/access-point-item.cpp
     menuitems/item.h
     menuitems/modem-info-item.cpp
     menuitems/section.h

=== modified file 'src/indicator/connectivity-service/connectivity-service.cpp'
--- src/indicator/connectivity-service/connectivity-service.cpp	2014-09-12 10:51:35 +0000
+++ src/indicator/connectivity-service/connectivity-service.cpp	2014-10-31 14:07:40 +0000
@@ -20,6 +20,8 @@
 #include "connectivity-service.h"
 #include <dbus-cpp/services/connectivity.h>
 
+#include <menumodel-cpp/gio-helpers/util.h>
+
 #include <core/dbus/asio/executor.h>
 #include <core/dbus/helper/type_mapper.h>
 
@@ -81,7 +83,7 @@
 };
 }
 
-class ConnectivityService::Private
+class ConnectivityService::Private : public std::enable_shared_from_this<Private>
 {
 public:
     std::thread m_connectivityServiceWorker;
@@ -105,6 +107,7 @@
 
     Private() = delete;
     Private(std::shared_ptr<networking::Manager> manager);
+    void ConstructL();
     ~Private();
 
     void updateNetworkingStatus();
@@ -114,9 +117,18 @@
 
 ConnectivityService::Private::Private(std::shared_ptr<networking::Manager> manager)
     : m_manager{manager}
+{}
+
+void
+ConnectivityService::Private::ConstructL()
 {
-    m_manager->characteristics().changed().connect(std::bind(&Private::updateNetworkingStatus, this));
-    m_manager->status().changed().connect(std::bind(&Private::updateNetworkingStatus, this));
+    auto that = shared_from_this();
+    m_manager->characteristics().changed().connect(
+                [that](int){ GMainLoopDispatch([that](){ that->updateNetworkingStatus(); }); });
+
+    typedef connectivity::networking::Manager::NetworkingStatus NetworkingStatus;
+    m_manager->status().changed().connect(
+                [that](NetworkingStatus){ GMainLoopDispatch([that](){ that->updateNetworkingStatus(); }); });
 
     m_bus = std::make_shared<core::dbus::Bus>(core::dbus::WellKnownBus::session);
 
@@ -185,7 +197,9 @@
     {
         auto reply = core::dbus::Message::make_method_return(msg);
         m_bus->send(reply);
-        m_unlockAllModems();
+
+        auto that = shared_from_this();
+        GMainLoopDispatch([that](){ that->m_unlockAllModems(); });
     });
 }
 
@@ -306,7 +320,9 @@
 
 ConnectivityService::ConnectivityService(std::shared_ptr<networking::Manager> manager)
     : d{new Private(manager)}
-{}
+{
+    d->ConstructL();
+}
 
 ConnectivityService::~ConnectivityService()
 {}

=== modified file 'src/indicator/connectivity-service/connectivity-service.h'
--- src/indicator/connectivity-service/connectivity-service.h	2014-08-07 23:00:39 +0000
+++ src/indicator/connectivity-service/connectivity-service.h	2014-10-31 14:07:40 +0000
@@ -29,11 +29,14 @@
     ConnectivityService(std::shared_ptr<connectivity::networking::Manager> manager);
     virtual ~ConnectivityService();
 
+    /**
+     * synced with GMainLoop
+     */
     core::Signal<> &unlockAllModems();
 
 private:
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 };
 
 #endif

=== modified file 'src/indicator/indicator-network-service.cpp'
--- src/indicator/indicator-network-service.cpp	2014-08-15 11:52:50 +0000
+++ src/indicator/indicator-network-service.cpp	2014-10-31 14:07:40 +0000
@@ -86,6 +86,8 @@
 
     std::shared_ptr<Service> menu {new Service(manager)};
     std::unique_ptr<ConnectivityService> connectivityService {new ConnectivityService(manager)};
+
+    // unlockAllModems is dispatched from GMainLoop
     connectivityService->unlockAllModems().connect([menu](){ menu->unlockAllModems(); });
 
     if (getenv("VALGRIND") != 0) {

=== renamed file 'src/indicator/menuitems/access-point-item.h' => 'src/indicator/menuitems/access-point-item.cpp'
--- src/indicator/menuitems/access-point-item.h	2014-08-15 11:49:44 +0000
+++ src/indicator/menuitems/access-point-item.cpp	2014-10-31 14:07:40 +0000
@@ -17,32 +17,42 @@
  *     Antti Kaijanmäki <[e-mail]>
  */
 
-#ifndef ACCESS_POINT_ITEM_H
-#define ACCESS_POINT_ITEM_H
+#include "access-point-item.h"
 
-#include "item.h"
 #include "menumodel-cpp/action.h"
 #include "menumodel-cpp/menu-item.h"
 #include "menumodel-cpp/gio-helpers/variant.h"
 
-#include <core/signal.h>
-
-#include <connectivity/networking/wifi/access-point.h>
 namespace networking = connectivity::networking;
 
 #include <functional>
 #include <vector>
 
-class AccessPointItem : public Item
+class AccessPointItem::Private : public std::enable_shared_from_this<Private>
 {
-
 public:
-    typedef std::shared_ptr<AccessPointItem> Ptr;
-
-    AccessPointItem() = delete;
-    explicit AccessPointItem(networking::wifi::AccessPoint::Ptr accessPoint, bool isActive = false)
-        : m_accessPoint{accessPoint},
+    AccessPointItem *q;
+
+    networking::wifi::AccessPoint::Ptr m_accessPoint;
+    bool m_isActive;
+
+    core::Signal<void> m_activated;
+
+    std::vector<core::Connection> m_connections;
+
+    Action::Ptr m_actionActivate;
+    Action::Ptr m_actionStrength;
+    MenuItem::Ptr m_item;
+
+
+    Private() = delete;
+    explicit Private(AccessPointItem *parent, networking::wifi::AccessPoint::Ptr accessPoint, bool isActive = false)
+        : q{parent},
+          m_accessPoint{accessPoint},
           m_isActive{isActive}
+    {}
+
+    void ConstructL()
     {
         static int id = 0;
         ++id; /// @todo guard me.
@@ -64,9 +74,17 @@
                                                     nullptr,
                                                     TypedVariant<std::uint8_t>(m_accessPoint->strength().get()));
 
-        auto con = m_accessPoint->strength().changed().connect(std::bind(&AccessPointItem::setStrength,
-                                                                         this,
-                                                                         std::placeholders::_1));
+        auto weak = std::weak_ptr<Private>(shared_from_this());
+        auto con = m_accessPoint->strength().changed().connect([weak](double value)
+        {
+            auto that = weak.lock();
+            if (!that)
+                return;
+            GMainLoopDispatch([that, value]()
+            {
+               that->setStrength(value);
+            });
+        });
         m_connections.push_back(con);
 
         m_actionActivate = std::make_shared<Action>(actionId,
@@ -79,11 +97,11 @@
             m_activated();
         });
 
-        m_actionGroup->add(m_actionActivate);
-        m_actionGroup->add(m_actionStrength);
+        q->actionGroup()->add(m_actionActivate);
+        q->actionGroup()->add(m_actionStrength);
     }
 
-    virtual ~AccessPointItem()
+    virtual ~Private()
     {
         for (auto con : m_connections)
             con.disconnect();
@@ -94,35 +112,32 @@
         /// @todo narrow_cast<>;
         m_actionStrength->setState(TypedVariant<std::uint8_t>(value));
     }
-
-    void setActive(bool value)
-    {
-        m_isActive = value;
-        m_actionActivate->setState(TypedVariant<bool>(m_isActive));
-    }
-
-    virtual MenuItem::Ptr
-    menuItem()
-    {
-        return m_item;
-    }
-
-    core::Signal<void> &activated()
-    {
-        return m_activated;
-    }
-
-private:
-    networking::wifi::AccessPoint::Ptr m_accessPoint;
-    bool m_isActive;
-
-    core::Signal<void> m_activated;
-
-    std::vector<core::Connection> m_connections;
-
-    Action::Ptr m_actionActivate;
-    Action::Ptr m_actionStrength;
-    MenuItem::Ptr m_item;
 };
 
-#endif // ACCESS_POINT_ITEM_H
+AccessPointItem::AccessPointItem(networking::wifi::AccessPoint::Ptr accessPoint, bool isActive)
+    : d{new Private(this, accessPoint, isActive)}
+{
+    d->ConstructL();
+}
+
+AccessPointItem::~AccessPointItem()
+{}
+
+void
+AccessPointItem::setActive(bool value)
+{
+    d->m_isActive = value;
+    d->m_actionActivate->setState(TypedVariant<bool>(d->m_isActive));
+}
+
+MenuItem::Ptr
+AccessPointItem::menuItem()
+{
+    return d->m_item;
+}
+
+core::Signal<void> &
+AccessPointItem::activated()
+{
+    return d->m_activated;
+}

=== added file 'src/indicator/menuitems/access-point-item.h'
--- src/indicator/menuitems/access-point-item.h	1970-01-01 00:00:00 +0000
+++ src/indicator/menuitems/access-point-item.h	2014-10-31 14:07:40 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2014 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Antti Kaijanmäki <[e-mail]>
+ */
+
+#ifndef ACCESS_POINT_ITEM_H
+#define ACCESS_POINT_ITEM_H
+
+#include <connectivity/networking/wifi/access-point.h>
+#include "item.h"
+
+class AccessPointItem : public Item
+{
+    class Private;
+    std::shared_ptr<Private> d;
+
+public:
+    typedef std::shared_ptr<AccessPointItem> Ptr;
+
+    AccessPointItem() = delete;
+    explicit AccessPointItem(connectivity::networking::wifi::AccessPoint::Ptr accessPoint, bool isActive = false);
+    virtual ~AccessPointItem();
+
+    void setActive(bool value);
+
+    virtual MenuItem::Ptr menuItem();
+    core::Signal<void> &activated();
+};
+
+#endif // ACCESS_POINT_ITEM_H

=== modified file 'src/indicator/modem-manager.cpp'
--- src/indicator/modem-manager.cpp	2014-10-15 11:31:13 +0000
+++ src/indicator/modem-manager.cpp	2014-10-31 14:07:40 +0000
@@ -19,6 +19,8 @@
 
 #include "modem-manager.h"
 
+#include <menumodel-cpp/gio-helpers/util.h>
+
 #include <notify-cpp/snapdecision/sim-unlock.h>
 
 #include "dbus-cpp/services/ofono.h"
@@ -30,7 +32,7 @@
 
 #include "sim-unlock-dialog.h"
 
-class ModemManager::Private
+class ModemManager::Private : public std::enable_shared_from_this<Private>
 {
 public:
 
@@ -47,6 +49,10 @@
     std::list<Modem::Ptr> m_pendingUnlocks;
 
     Private()
+    {}
+
+    void
+    ConstructL()
     {
         m_bus = std::make_shared<core::dbus::Bus>(core::dbus::WellKnownBus::system);
 
@@ -83,8 +89,10 @@
             ofono_disappeared();
         }
         m_watcher = m_dbus->make_service_watcher(org::ofono::Service::name());
-        m_watcher->service_registered().connect([this]()  { ofono_appeared();    });
-        m_watcher->service_unregistered().connect([this](){ ofono_disappeared(); });
+
+        auto that = shared_from_this();
+        m_watcher->service_registered().connect([that](){ GMainLoopDispatch([that](){ that->ofono_appeared(); }); });
+        m_watcher->service_unregistered().connect([that](){ GMainLoopDispatch([that](){ that->ofono_disappeared(); }); });
     }
 
     ~Private()
@@ -109,7 +117,10 @@
             std::cerr << e.what() << std::endl;
         }
 
-        m_ofono->manager->modems.changed().connect(std::bind(&Private::modems_changed, this, std::placeholders::_1));
+        auto that = shared_from_this();
+        m_ofono->manager->modems.changed().connect(
+                    [that](std::map<core::dbus::types::ObjectPath, org::ofono::Interface::Modem::Ptr> modems)
+        { GMainLoopDispatch([that, modems]() { that->modems_changed(modems); }); });
         modems_changed(m_ofono->manager->modems.get());
     }
 
@@ -171,8 +182,9 @@
 };
 
 ModemManager::ModemManager()
+    : d{new Private}
 {
-    d.reset(new Private);
+    d->ConstructL();
 }
 
 ModemManager::~ModemManager()

=== modified file 'src/indicator/modem-manager.h'
--- src/indicator/modem-manager.h	2014-08-07 20:24:48 +0000
+++ src/indicator/modem-manager.h	2014-10-31 14:07:40 +0000
@@ -28,7 +28,7 @@
 class ModemManager
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
 
@@ -37,9 +37,19 @@
     ModemManager();
     ~ModemManager();
 
+    /**
+     * must be called from GMainLoop
+     */
     void unlockModem(Modem::Ptr modem);
+
+    /**
+     * must be called from GMainLoop
+     */
     void unlockAllModems();
 
+    /**
+     * changed() emitted from GMainLoop
+     */
     const core::Property<std::set<Modem::Ptr>> &modems();
 };
 

=== modified file 'src/indicator/modem.cpp'
--- src/indicator/modem.cpp	2014-10-08 09:45:57 +0000
+++ src/indicator/modem.cpp	2014-10-31 14:07:40 +0000
@@ -19,7 +19,9 @@
 
 #include "modem.h"
 
-class Modem::Private
+#include <menumodel-cpp/gio-helpers/util.h>
+
+class Modem::Private : public std::enable_shared_from_this<Private>
 {
 public:
     core::Property<bool> m_online;
@@ -41,8 +43,60 @@
     void simManagerChanged(org::ofono::Interface::SimManager::Ptr simmgr);
 
     void update();
+
+    Private() = delete;
+    Private(org::ofono::Interface::Modem::Ptr ofonoModem);
+    void ConstructL();
 };
 
+Modem::Private::Private(org::ofono::Interface::Modem::Ptr ofonoModem)
+    : m_ofonoModem{ofonoModem}
+{}
+
+void
+Modem::Private::ConstructL()
+{
+    auto that = shared_from_this();
+
+    m_online.set(m_ofonoModem->online.get());
+    m_ofonoModem->online.changed().connect([that](bool value){
+        GMainLoopDispatch([that, value]() {
+            that->m_online.set(value);
+        });
+    });
+
+    simManagerChanged(m_ofonoModem->simManager.get());
+    m_ofonoModem->simManager.changed().connect([that](org::ofono::Interface::SimManager::Ptr simmgr)
+    {
+        GMainLoopDispatch([that, simmgr](){
+            that->simManagerChanged(simmgr);
+        });
+    });
+
+    networkRegistrationChanged(m_ofonoModem->networkRegistration.get());
+    m_ofonoModem->networkRegistration.changed().connect([that](org::ofono::Interface::NetworkRegistration::Ptr netreg)
+    {
+        GMainLoopDispatch([that, netreg](){
+            that->networkRegistrationChanged(netreg);
+        });
+    });
+
+    /// @todo hook up with system-settings to allow changing the identifier.
+    ///       for now just provide the defaults
+    const auto path = m_ofonoModem->object->path().as_string();
+    if (path == "/ril_0") {
+        m_simIdentifier.set("SIM 1");
+        m_index = 1;
+    } else if (path == "/ril_1") {
+        m_simIdentifier.set("SIM 2");
+        m_index = 2;
+    } else {
+        m_simIdentifier.set(path);
+    }
+
+}
+
+
 void
 Modem::Private::update()
 {
@@ -119,10 +173,38 @@
 Modem::Private::networkRegistrationChanged(org::ofono::Interface::NetworkRegistration::Ptr netreg)
 {
     if (netreg) {
-        netreg->operatorName.changed().connect(std::bind(&Private::update, this));
-        netreg->status.changed().connect(std::bind(&Private::update, this));
-        netreg->strength.changed().connect(std::bind(&Private::update, this));
-        netreg->technology.changed().connect(std::bind(&Private::update, this));
+        auto that = shared_from_this();
+        netreg->operatorName.changed().connect([that](const std::string &)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+        netreg->status.changed().connect([that](org::ofono::Interface::NetworkRegistration::Status)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
+        netreg->strength.changed().connect([that](std::int8_t)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
+        netreg->technology.changed().connect([that](org::ofono::Interface::NetworkRegistration::Technology)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
     }
     update();
 }
@@ -132,41 +214,39 @@
 Modem::Private::simManagerChanged(org::ofono::Interface::SimManager::Ptr simmgr)
 {
     if (simmgr) {
-        simmgr->present.changed().connect(std::bind(&Private::update, this));
-        simmgr->pinRequired.changed().connect(std::bind(&Private::update, this));
-        simmgr->retries.changed().connect(std::bind(&Private::update, this));
+        auto that = shared_from_this();
+        simmgr->present.changed().connect([that](bool)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
+        simmgr->pinRequired.changed().connect([that](org::ofono::Interface::SimManager::PinType)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
+        simmgr->retries.changed().connect([that](std::map<org::ofono::Interface::SimManager::PinType, std::uint8_t>)
+        {
+            GMainLoopDispatch([that]()
+            {
+                that->update();
+            });
+        });
+
     }
     update();
 }
 
 Modem::Modem(org::ofono::Interface::Modem::Ptr ofonoModem)
+    : d{new Private(ofonoModem)}
 {
-    d.reset(new Private);
-    d->m_ofonoModem = ofonoModem;
-
-    d->m_online.set(d->m_ofonoModem->online.get());
-    d->m_ofonoModem->online.changed().connect([this](bool value){
-        d->m_online.set(value);
-    });
-
-    d->simManagerChanged(d->m_ofonoModem->simManager.get());
-    d->m_ofonoModem->simManager.changed().connect(std::bind(&Private::simManagerChanged, d.get(), std::placeholders::_1));
-
-    d->networkRegistrationChanged(d->m_ofonoModem->networkRegistration.get());
-    d->m_ofonoModem->networkRegistration.changed().connect(std::bind(&Private::networkRegistrationChanged, d.get(), std::placeholders::_1));
-
-    /// @todo hook up with system-settings to allow changing the identifier.
-    ///       for now just provide the defaults
-    const auto path = ofonoModem->object->path().as_string();
-    if (path == "/ril_0") {
-        d->m_simIdentifier.set("SIM 1");
-        d->m_index = 1;
-    } else if (path == "/ril_1") {
-        d->m_simIdentifier.set("SIM 2");
-        d->m_index = 2;
-    } else {
-        d->m_simIdentifier.set(path);
-    }
+    d->ConstructL();
 }
 
 Modem::~Modem()

=== modified file 'src/indicator/modem.h'
--- src/indicator/modem.h	2014-10-02 11:36:00 +0000
+++ src/indicator/modem.h	2014-10-31 14:07:40 +0000
@@ -27,10 +27,13 @@
 
 #include "dbus-cpp/services/ofono.h"
 
+/**
+ * all signals and property changes emitted from GMainLoop
+ */
 class Modem
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
     enum class PinType

=== modified file 'src/indicator/quick-access-section.cpp'
--- src/indicator/quick-access-section.cpp	2014-10-09 10:58:30 +0000
+++ src/indicator/quick-access-section.cpp	2014-10-31 14:07:40 +0000
@@ -26,7 +26,7 @@
 
 namespace networking = connectivity::networking;
 
-class QuickAccessSection::Private
+class QuickAccessSection::Private : public std::enable_shared_from_this<Private>
 {
 public:
     ActionGroupMerger::Ptr m_actionGroupMerger;
@@ -36,15 +36,23 @@
 
     SwitchItem::Ptr m_flightModeSwitch;
 
+    Private() = delete;
     Private(std::shared_ptr<networking::Manager> manager);
+    void ConstructL();
 };
 
 QuickAccessSection::Private::Private(std::shared_ptr<networking::Manager> manager)
     : m_manager{manager}
+{}
+
+void
+QuickAccessSection::Private::ConstructL()
 {
     m_actionGroupMerger = std::make_shared<ActionGroupMerger>();
     m_menu = std::make_shared<Menu>();
 
+    auto that = shared_from_this();
+
     m_flightModeSwitch = std::make_shared<SwitchItem>(_("Flight Mode"), "airplane", "enabled");
     switch (m_manager->flightMode().get()) {
     case networking::Manager::FlightModeStatus::off:
@@ -54,15 +62,18 @@
         m_flightModeSwitch->state().set(true);
         break;
     }
-    m_manager->flightMode().changed().connect([this](networking::Manager::FlightModeStatus value){
-        switch (value) {
-        case networking::Manager::FlightModeStatus::off:
-            m_flightModeSwitch->state().set(false);
-            break;
-        case networking::Manager::FlightModeStatus::on:
-            m_flightModeSwitch->state().set(true);
-            break;
-        }
+    m_manager->flightMode().changed().connect([that](networking::Manager::FlightModeStatus value){
+        GMainLoopDispatch([that, value]
+        {
+            switch (value) {
+            case networking::Manager::FlightModeStatus::off:
+                that->m_flightModeSwitch->state().set(false);
+                break;
+            case networking::Manager::FlightModeStatus::on:
+                that->m_flightModeSwitch->state().set(true);
+                break;
+            }
+        });
     });
     m_flightModeSwitch->activated().connect([this](){
         if (m_flightModeSwitch->state().get()) {
@@ -85,8 +96,9 @@
 }
 
 QuickAccessSection::QuickAccessSection(std::shared_ptr<networking::Manager> manager)
+    : d{new Private(manager)}
 {
-    d.reset(new Private(manager));
+    d->ConstructL();
 }
 
 QuickAccessSection::~QuickAccessSection()

=== modified file 'src/indicator/quick-access-section.h'
--- src/indicator/quick-access-section.h	2014-04-23 14:41:46 +0000
+++ src/indicator/quick-access-section.h	2014-10-31 14:07:40 +0000
@@ -23,10 +23,13 @@
 #include "menuitems/section.h"
 #include <connectivity/networking/manager.h>
 
+/**
+ * all signals and property changes emitted from GMainLoop
+ */
 class QuickAccessSection : public Section
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
     typedef std::shared_ptr<QuickAccessSection> Ptr;

=== modified file 'src/indicator/root-state.cpp'
--- src/indicator/root-state.cpp	2014-10-06 08:30:34 +0000
+++ src/indicator/root-state.cpp	2014-10-31 14:07:40 +0000
@@ -24,9 +24,11 @@
 #include <connectivity/networking/wifi/link.h>
 #include <connectivity/networking/wifi/access-point.h>
 
+#include <menumodel-cpp/gio-helpers/util.h>
+
 namespace networking = connectivity::networking;
 
-class RootState::Private
+class RootState::Private : public std::enable_shared_from_this<Private>
 {
 public:
     std::shared_ptr<networking::Manager> m_manager;
@@ -43,6 +45,7 @@
 
     Private() = delete;
     Private(std::shared_ptr<networking::Manager> manager, ModemManager::Ptr modemManager);
+    void ConstructL();
 
     void modemsChanged(const std::set<Modem::Ptr> &modems);
 
@@ -57,14 +60,38 @@
 RootState::Private::Private(std::shared_ptr<networking::Manager> manager, ModemManager::Ptr modemManager)
     : m_manager{manager},
       m_modemManager{modemManager}
-{    
-    m_manager->flightMode().changed().connect(std::bind(&Private::updateRootState, this));
-
+{}
+
+void
+RootState::Private::ConstructL()
+{
+    auto that = shared_from_this();
+
+    m_manager->flightMode().changed().connect([that](connectivity::networking::Manager::FlightModeStatus)
+    {
+        GMainLoopDispatch([that](){
+            that->updateRootState();
+        });
+    });
+
+    modemsChanged(m_modemManager->modems().get());
+    // modem properties and signals already synced with GMainLoop
     m_modemManager->modems().changed().connect(std::bind(&Private::modemsChanged, this, std::placeholders::_1));
-    modemsChanged(m_modemManager->modems().get());
-
-    m_manager->status().changed().connect(std::bind(&Private::updateNetworkingIcon, this));
-    m_manager->links().changed().connect(std::bind(&Private::updateNetworkingIcon, this));
+
+    m_manager->status().changed().connect([that](connectivity::networking::Manager::NetworkingStatus)
+    {
+        GMainLoopDispatch([that](){
+            that->updateNetworkingIcon();
+        });
+    });
+
+    m_manager->links().changed().connect([that](std::set<connectivity::networking::Link::Ptr>)
+    {
+        GMainLoopDispatch([that](){
+            that->updateNetworkingIcon();
+        });
+    });
+
 
     // will also call updateRootState()
     updateNetworkingIcon();
@@ -90,6 +117,7 @@
         m_cellularIcons.erase(modem);
 
     for (auto modem : added) {
+        // modem properties and signals already synced with GMainLoop
         modem->online().changed().connect(std::bind(&Private::updateModem, this, Modem::WeakPtr(modem)));
         modem->simStatus().changed().connect(std::bind(&Private::updateModem, this, Modem::WeakPtr(modem)));
         modem->status().changed().connect(std::bind(&Private::updateModem, this, Modem::WeakPtr(modem)));
@@ -319,8 +347,9 @@
 }
 
 RootState::RootState(std::shared_ptr<connectivity::networking::Manager> manager, ModemManager::Ptr modemManager)
+    : d{new Private(manager, modemManager)}
 {
-    d.reset(new Private(manager, modemManager));
+    d->ConstructL();
 }
 
 RootState::~RootState()

=== modified file 'src/indicator/root-state.h'
--- src/indicator/root-state.h	2014-05-12 11:36:59 +0000
+++ src/indicator/root-state.h	2014-10-31 14:07:40 +0000
@@ -25,10 +25,13 @@
 
 #include "menumodel-cpp/gio-helpers/variant.h"
 
+/**
+ * all signals and property changes emitted from GMainLoop
+ */
 class RootState
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
     typedef std::shared_ptr<RootState> Ptr;

=== modified file 'src/indicator/sim-unlock-dialog.h'
--- src/indicator/sim-unlock-dialog.h	2014-10-14 19:54:51 +0000
+++ src/indicator/sim-unlock-dialog.h	2014-10-31 14:07:40 +0000
@@ -23,6 +23,9 @@
 #include <memory>
 #include "modem.h"
 
+/**
+ * all signals and property changes dispatched from GMainLoop
+ */
 class SimUnlockDialog
 {
     class Private;

=== renamed file 'src/indicator/wifi-link-item.h' => 'src/indicator/wifi-link-item.cpp'
--- src/indicator/wifi-link-item.h	2014-09-24 23:37:18 +0000
+++ src/indicator/wifi-link-item.cpp	2014-10-31 14:07:40 +0000
@@ -17,26 +17,26 @@
  *     Antti Kaijanmäki <[e-mail]>
  */
 
-#ifndef WIFI_LINK_ITEM_H
-#define WIFI_LINK_ITEM_H
+#include "wifi-link-item.h"
 
-#include "menuitems/switch-item.h"
+#include "menuitems/text-item.h"
 #include "menuitems/access-point-item.h"
-#include "menuitems/text-item.h"
 
 #include "menumodel-cpp/action-group.h"
 #include "menumodel-cpp/action-group-merger.h"
 #include "menumodel-cpp/menu.h"
 #include "menumodel-cpp/menu-merger.h"
 
-#include <connectivity/networking/wifi/link.h>
 namespace networking = connectivity::networking;
 
 #include <algorithm>
 #include <locale>
 
-class WifiLinkItem : public Item
+class WifiLinkItem::Private : public std::enable_shared_from_this<Private>
 {
+public:
+    ActionGroupMerger::Ptr m_actionGroupMerger;
+
     networking::wifi::Link::Ptr m_link;
 
     /// @todo do something with me...
@@ -66,13 +66,18 @@
     std::mutex m_updateActiveAccessPointMutex;
 
 public:
-    typedef std::shared_ptr<WifiLinkItem> Ptr;
-
-    WifiLinkItem() = delete;
-    virtual ~WifiLinkItem() = default;
-    WifiLinkItem(networking::wifi::Link::Ptr link)
+
+
+    Private() = delete;
+    ~Private() {}
+    Private(networking::wifi::Link::Ptr link)
         : m_link {link}
+    {}
+
+    void ConstructL()
     {
+        m_actionGroupMerger = std::make_shared<ActionGroupMerger>();
+
         m_topMenu = std::make_shared<Menu>();
 
         m_connectedBeforeApsMenu = std::make_shared<Menu>();
@@ -103,14 +108,24 @@
             return a_upper < b_upper;
         };
 
+        auto that = shared_from_this();
+
         updateAccessPoints(m_link->accessPoints().get());
-        m_link->accessPoints().changed().connect([this](std::set<networking::wifi::AccessPoint::Ptr> accessPoints){
-            updateAccessPoints(accessPoints);
+        m_link->accessPoints().changed().connect([that](std::set<networking::wifi::AccessPoint::Ptr> accessPoints)
+        {
+            GMainLoopDispatch([that, accessPoints]()
+            {
+                that->updateAccessPoints(accessPoints);
+            });
         });
 
         updateActiveAccessPoint(m_link->activeAccessPoint().get());
-        m_link->activeAccessPoint().changed().connect([this](networking::wifi::AccessPoint::Ptr ap){
-            updateActiveAccessPoint(ap);
+        m_link->activeAccessPoint().changed().connect([that](networking::wifi::AccessPoint::Ptr ap)
+        {
+            GMainLoopDispatch([that, ap]()
+            {
+                that->updateActiveAccessPoint(ap);
+            });
         });
 
         m_otherNetwork = std::make_shared<TextItem>(_("Other network…"), "wifi", "othernetwork");
@@ -204,12 +219,26 @@
         }
     }
 
-    virtual MenuItem::Ptr
-    menuItem() {
-        return m_item;
-    }
-
-
 };
 
-#endif // WIFI_LINK_ITEM_H
+
+WifiLinkItem::WifiLinkItem(connectivity::networking::wifi::Link::Ptr link)
+    : d{new Private(link)}
+{
+    d->ConstructL();
+}
+
+WifiLinkItem::~WifiLinkItem()
+{}
+
+ActionGroup::Ptr
+WifiLinkItem::actionGroup()
+{
+    return d->m_actionGroupMerger->actionGroup();
+}
+
+MenuItem::Ptr
+WifiLinkItem::menuItem()
+{
+    return d->m_item;
+}

=== added file 'src/indicator/wifi-link-item.h'
--- src/indicator/wifi-link-item.h	1970-01-01 00:00:00 +0000
+++ src/indicator/wifi-link-item.h	2014-10-31 14:07:40 +0000
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2014 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Antti Kaijanmäki <[e-mail]>
+ */
+
+#ifndef WIFI_LINK_ITEM_H
+#define WIFI_LINK_ITEM_H
+
+#include <connectivity/networking/wifi/link.h>
+
+#include "menuitems/item.h"
+
+class WifiLinkItem : public Item
+{
+    class Private;
+    std::shared_ptr<Private> d;
+
+public:
+    typedef std::shared_ptr<WifiLinkItem> Ptr;
+
+    WifiLinkItem() = delete;
+    WifiLinkItem(connectivity::networking::wifi::Link::Ptr link);
+    virtual ~WifiLinkItem();
+
+    virtual MenuItem::Ptr menuItem();
+    virtual ActionGroup::Ptr actionGroup();
+};
+
+#endif // WIFI_LINK_ITEM_H

=== modified file 'src/indicator/wifi-section.cpp'
--- src/indicator/wifi-section.cpp	2014-09-25 18:48:39 +0000
+++ src/indicator/wifi-section.cpp	2014-10-31 14:07:40 +0000
@@ -23,7 +23,17 @@
 
 #include "url-dispatcher-cpp/url-dispatcher.h"
 
-class WifiSection::Private
+#include "menuitems/switch-item.h"
+#include "menuitems/text-item.h"
+
+#include "menumodel-cpp/action-group.h"
+#include "menumodel-cpp/action-group-merger.h"
+#include "menumodel-cpp/menu.h"
+#include "menumodel-cpp/menu-merger.h"
+
+using namespace connectivity;
+
+class WifiSection::Private : public std::enable_shared_from_this<Private>
 {
 public:
     std::shared_ptr<connectivity::networking::Manager> m_manager;
@@ -40,6 +50,10 @@
     Private() = delete;
     Private(std::shared_ptr<connectivity::networking::Manager> manager)
         : m_manager{manager}
+    {}
+
+    void
+    ConstructL()
     {
         m_actionGroupMerger = std::make_shared<ActionGroupMerger>();
         m_menu = std::make_shared<Menu>();
@@ -56,9 +70,14 @@
             m_settingsMenu->append(*m_switch);
         }
 
+        auto that = shared_from_this();
+
         m_switch->state().set(m_manager->wifiEnabled().get());
-        m_manager->wifiEnabled().changed().connect([this](bool value) {
-            m_switch->state().set(value);
+        m_manager->wifiEnabled().changed().connect([that](bool value) {
+            GMainLoopDispatch([that, value]()
+            {
+                that->m_switch->state().set(value);
+            });
         });
         m_switch->activated().connect([this](){
             if (m_switch->state().get()) {
@@ -73,8 +92,14 @@
         });
 
 
-        m_manager->links().changed().connect(std::bind(&Private::updateLinks, this));
         updateLinks();
+        m_manager->links().changed().connect([that](std::set<networking::Link::Ptr>)
+        {
+            GMainLoopDispatch([that]()
+            {
+                    that->updateLinks();
+            });
+        });
 
         m_openWifiSettings = std::make_shared<TextItem>(_("Wi-Fi settings…"), "wifi", "settings");
         m_openWifiSettings->activated().connect([](){
@@ -115,14 +140,13 @@
 };
 
 WifiSection::WifiSection(std::shared_ptr<connectivity::networking::Manager> manager)
+    : d{new Private(manager)}
 {
-    d.reset(new Private(manager));
+    d->ConstructL();
 }
 
 WifiSection::~WifiSection()
-{
-
-}
+{}
 
 ActionGroup::Ptr
 WifiSection::actionGroup()

=== modified file 'src/indicator/wifi-section.h'
--- src/indicator/wifi-section.h	2014-08-15 11:49:44 +0000
+++ src/indicator/wifi-section.h	2014-10-31 14:07:40 +0000
@@ -27,7 +27,7 @@
 class WifiSection : public Section
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
     typedef std::shared_ptr<WifiSection> Ptr;

=== modified file 'src/indicator/wwan-link-item.cpp'
--- src/indicator/wwan-link-item.cpp	2014-10-02 11:36:00 +0000
+++ src/indicator/wwan-link-item.cpp	2014-10-31 14:07:40 +0000
@@ -23,7 +23,7 @@
 
 #include "menumodel-cpp/menu.h"
 
-class WwanLinkItem::Private
+class WwanLinkItem::Private : public std::enable_shared_from_this<Private>
 {
 public:
 
@@ -39,6 +39,7 @@
 
     Private() = delete;
     Private(Modem::Ptr modem, ModemManager::Ptr modemManager);
+    void ConstructL();
 
     void update();
     void unlockSim();
@@ -47,6 +48,10 @@
 WwanLinkItem::Private::Private(Modem::Ptr modem, ModemManager::Ptr modemManager)
     : m_modem{modem},
       m_modemManager{modemManager}
+{}
+
+void
+WwanLinkItem::Private::ConstructL()
 {
     m_actionGroupMerger = std::make_shared<ActionGroupMerger>();
     m_menu = std::make_shared<Menu>();
@@ -60,6 +65,7 @@
     m_showIdentifier.set(false);
     m_showIdentifier.changed().connect(std::bind(&Private::update, this));
 
+    // already synced with GMainLoop
     m_modem->online().changed().connect(std::bind(&Private::update, this));
     m_modem->simStatus().changed().connect(std::bind(&Private::update, this));
     m_modem->operatorName().changed().connect(std::bind(&Private::update, this));
@@ -152,8 +158,10 @@
 }
 
 WwanLinkItem::WwanLinkItem(Modem::Ptr modem, ModemManager::Ptr modemManager)
+    : d{new Private(modem, modemManager)}
+
 {
-    d.reset(new Private(modem, modemManager));
+    d->ConstructL();
 }
 
 WwanLinkItem::~WwanLinkItem()

=== modified file 'src/indicator/wwan-section.cpp'
--- src/indicator/wwan-section.cpp	2014-08-15 10:47:02 +0000
+++ src/indicator/wwan-section.cpp	2014-10-31 14:07:40 +0000
@@ -53,12 +53,17 @@
 
     Private() = delete;
     Private(ModemManager::Ptr modemManager);
+    void ConstructL();
 
     void modemsChanged(const std::set<Modem::Ptr> &modems);
 };
 
 WwanSection::Private::Private(ModemManager::Ptr modemManager)
     : m_modemManager{modemManager}
+{}
+
+void
+WwanSection::Private::ConstructL()
 {
     m_actionGroupMerger = std::make_shared<ActionGroupMerger>();
     m_menuMerger = std::make_shared<MenuMerger>();
@@ -85,6 +90,7 @@
     });
     m_actionGroupMerger->add(*m_openCellularSettings);
 
+    // already synced with GMainLoop
     m_modemManager->modems().changed().connect(std::bind(&Private::modemsChanged, this, std::placeholders::_1));
     modemsChanged(m_modemManager->modems());
 }
@@ -149,8 +155,9 @@
 }
 
 WwanSection::WwanSection(ModemManager::Ptr modemManager)
+    : d{new Private(modemManager)}
 {
-    d.reset(new Private(modemManager));
+    d->ConstructL();
 }
 
 WwanSection::~WwanSection()

=== modified file 'src/indicator/wwan-section.h'
--- src/indicator/wwan-section.h	2014-08-15 11:52:50 +0000
+++ src/indicator/wwan-section.h	2014-10-31 14:07:40 +0000
@@ -26,7 +26,7 @@
 class WwanSection : public Section
 {
     class Private;
-    std::unique_ptr<Private> d;
+    std::shared_ptr<Private> d;
 
 public:
     typedef std::shared_ptr<WwanSection> Ptr;

=== modified file 'src/menumodel-cpp/gio-helpers/util.cpp'
--- src/menumodel-cpp/gio-helpers/util.cpp	2014-09-26 13:07:27 +0000
+++ src/menumodel-cpp/gio-helpers/util.cpp	2014-10-31 14:07:40 +0000
@@ -25,28 +25,28 @@
 gboolean
 GMainLoopDispatch::dispatch_cb(gpointer)
 {
-    std::lock_guard<std::mutex> lock(_lock);
+    std::unique_lock<std::mutex> lock(_lock);
     for (auto func : _funcs) {
+        lock.unlock();
         (*func)();
+        lock.lock();
         delete func;
     }
     _funcs.clear();
     return G_SOURCE_REMOVE;
 }
 
-GMainLoopDispatch::GMainLoopDispatch(std::function<void()> func)
+GMainLoopDispatch::GMainLoopDispatch(Func func)
 {
+    std::unique_lock<std::mutex> lock(_lock);
+
     if (g_main_context_acquire(g_main_context_default())) {
-        if (_funcs.empty())
-            func();
-        else {
-            std::function<void()> *funcPtr = new std::function<void()>(func);
-            _funcs.push_back(funcPtr);
-        }
+        // already running inside GMainLoop..
+        // free the lock and dispatch immediately.
+        lock.unlock();
+        func();
         g_main_context_release(g_main_context_default());
     } else {
-        std::lock_guard<std::mutex> lock(_lock);
-
         std::function<void()> *funcPtr = new std::function<void()>(func);
         if (_funcs.empty()) {
             g_idle_add_full(G_PRIORITY_HIGH,

=== modified file 'src/menumodel-cpp/gio-helpers/util.h'
--- src/menumodel-cpp/gio-helpers/util.h	2014-09-26 13:07:27 +0000
+++ src/menumodel-cpp/gio-helpers/util.h	2014-10-31 14:07:40 +0000
@@ -54,7 +54,7 @@
             g_main_context_release(g_main_context_default());
         } else {
             std::unique_lock<std::mutex> lk(m_mutex);
-            g_idle_add_full(G_PRIORITY_HIGH,
+            g_idle_add_full(G_PRIORITY_HIGH + 1,
                             GSourceFunc(GMainLoopSync::dispatch_cb),
                             this,
                             nullptr);
@@ -77,7 +77,7 @@
     static std::list<Func *> _funcs;
 
     //GMainLoopDispatch() = delete;
-    GMainLoopDispatch(std::function<void()> func);
+    GMainLoopDispatch(Func func);
 };
 
 struct GObjectDeleter {

=== modified file 'tests/unit/indicator/menuitems/CMakeLists.txt'
--- tests/unit/indicator/menuitems/CMakeLists.txt	2014-08-19 21:03:04 +0000
+++ tests/unit/indicator/menuitems/CMakeLists.txt	2014-10-31 14:07:40 +0000
@@ -2,6 +2,7 @@
 set(
     MENUITEMS_CPP_UNIT_TESTS_SRC
     test-access-point-item.cpp
+    ${CMAKE_SOURCE_DIR}/src/indicator/menuitems/access-point-item.cpp
     test-switch-item.cpp
 )
 

=== modified file 'tests/unit/indicator/menuitems/test-access-point-item.cpp'
--- tests/unit/indicator/menuitems/test-access-point-item.cpp	2014-05-12 11:48:42 +0000
+++ tests/unit/indicator/menuitems/test-access-point-item.cpp	2014-10-31 14:07:40 +0000
@@ -30,6 +30,8 @@
 using namespace QtDBusTest;
 using namespace testutils;
 
+using namespace connectivity;
+
 namespace
 {