Merge lp:~3v1n0/unity/cancellable-gnome-session-manager into lp:unity

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 4219
Proposed branch: lp:~3v1n0/unity/cancellable-gnome-session-manager
Merge into: lp:unity
Diff against target: 189 lines (+24/-20)
5 files modified
UnityCore/GLibDBusProxy.cpp (+12/-8)
UnityCore/GLibDBusProxy.h (+2/-2)
UnityCore/GnomeSessionManager.cpp (+9/-9)
UnityCore/GnomeSessionManagerImpl.h (+1/-0)
unity-shared/SystemdWrapper.cpp (+0/-1)
To merge this branch: bzr merge lp:~3v1n0/unity/cancellable-gnome-session-manager
Reviewer Review Type Date Requested Status
Eleni Maria Stea (community) Approve
Review via email: mp+314925@code.launchpad.net

Commit message

GnomeSessionManager: add gcancellable to instance and use it for calls with temporary proxies

This fixes various crashes when the session manager is destroyed while a temporary proxy
call is still in progress, and the callback is called afterwards.

To post a comment you must log in.
Revision history for this message
Eleni Maria Stea (hikiko) wrote :

+1 :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'UnityCore/GLibDBusProxy.cpp'
--- UnityCore/GLibDBusProxy.cpp 2015-11-24 16:40:03 +0000
+++ UnityCore/GLibDBusProxy.cpp 2017-01-17 15:51:00 +0000
@@ -609,7 +609,7 @@
609 return nullptr;609 return nullptr;
610}610}
611611
612void DBusProxy::GetProperty(std::string const& name, ReplyCallback const& callback)612void DBusProxy::GetProperty(std::string const& name, ReplyCallback const& callback, GCancellable *cancellable)
613{613{
614 if (!callback)614 if (!callback)
615 return;615 return;
@@ -620,7 +620,8 @@
620 pimpl->name_.c_str(), pimpl->object_path_.c_str(),620 pimpl->name_.c_str(), pimpl->object_path_.c_str(),
621 "org.freedesktop.DBus.Properties",621 "org.freedesktop.DBus.Properties",
622 "Get", g_variant_new ("(ss)", pimpl->interface_name_.c_str(), name.c_str()),622 "Get", g_variant_new ("(ss)", pimpl->interface_name_.c_str(), name.c_str()),
623 G_VARIANT_TYPE("(v)"), G_DBUS_CALL_FLAGS_NONE, -1, pimpl->cancellable_,623 G_VARIANT_TYPE("(v)"), G_DBUS_CALL_FLAGS_NONE, -1,
624 cancellable ? cancellable : pimpl->cancellable_,
624 [] (GObject *source, GAsyncResult *res, gpointer user_data) {625 [] (GObject *source, GAsyncResult *res, gpointer user_data) {
625 glib::Error err;626 glib::Error err;
626 std::unique_ptr<ReplyCallback> callback(static_cast<ReplyCallback*>(user_data));627 std::unique_ptr<ReplyCallback> callback(static_cast<ReplyCallback*>(user_data));
@@ -641,15 +642,16 @@
641 else642 else
642 {643 {
643 // This will get the property as soon as we have a connection644 // This will get the property as soon as we have a connection
645 glib::Object<GCancellable> canc(cancellable, AddRef());
644 auto conn = std::make_shared<sigc::connection>();646 auto conn = std::make_shared<sigc::connection>();
645 *conn = connected.connect([this, conn, name, callback] {647 *conn = connected.connect([this, conn, name, callback, canc] {
646 GetProperty(name, callback);648 GetProperty(name, callback, canc);
647 conn->disconnect();649 conn->disconnect();
648 });650 });
649 }651 }
650}652}
651653
652void DBusProxy::SetProperty(std::string const& name, GVariant* value)654void DBusProxy::SetProperty(std::string const& name, GVariant* value, GCancellable *cancellable)
653{655{
654 if (IsConnected())656 if (IsConnected())
655 {657 {
@@ -657,7 +659,8 @@
657 pimpl->name_.c_str(), pimpl->object_path_.c_str(),659 pimpl->name_.c_str(), pimpl->object_path_.c_str(),
658 "org.freedesktop.DBus.Properties",660 "org.freedesktop.DBus.Properties",
659 "Set", g_variant_new ("(ssv)", pimpl->interface_name_.c_str(), name.c_str(), value),661 "Set", g_variant_new ("(ssv)", pimpl->interface_name_.c_str(), name.c_str(), value),
660 nullptr, G_DBUS_CALL_FLAGS_NONE, -1, pimpl->cancellable_,662 nullptr, G_DBUS_CALL_FLAGS_NONE, -1,
663 cancellable ? cancellable : pimpl->cancellable_,
661 [] (GObject *source, GAsyncResult *res, gpointer user_data) {664 [] (GObject *source, GAsyncResult *res, gpointer user_data) {
662 glib::Error err;665 glib::Error err;
663 Variant result(g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &err), StealRef());666 Variant result(g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &err), StealRef());
@@ -670,9 +673,10 @@
670 else673 else
671 {674 {
672 // This will set the property as soon as we have a connection675 // This will set the property as soon as we have a connection
676 glib::Object<GCancellable> canc(cancellable, AddRef());
673 auto conn = std::make_shared<sigc::connection>();677 auto conn = std::make_shared<sigc::connection>();
674 *conn = connected.connect([this, conn, name, value] {678 *conn = connected.connect([this, conn, name, value, canc] {
675 SetProperty(name, value);679 SetProperty(name, value, canc);
676 conn->disconnect();680 conn->disconnect();
677 });681 });
678 }682 }
679683
=== modified file 'UnityCore/GLibDBusProxy.h'
--- UnityCore/GLibDBusProxy.h 2014-10-10 11:35:40 +0000
+++ UnityCore/GLibDBusProxy.h 2017-01-17 15:51:00 +0000
@@ -74,8 +74,8 @@
74 bool IsConnected() const;74 bool IsConnected() const;
7575
76 Variant GetProperty(std::string const& property_name) const;76 Variant GetProperty(std::string const& property_name) const;
77 void GetProperty(std::string const& property_name, ReplyCallback const&);77 void GetProperty(std::string const& property_name, ReplyCallback const&, GCancellable *cancellable = nullptr);
78 void SetProperty(std::string const& property_name, GVariant* value);78 void SetProperty(std::string const& property_name, GVariant* value, GCancellable *cancellable = nullptr);
7979
80 void Connect(std::string const& signal_name, ReplyCallback const& callback);80 void Connect(std::string const& signal_name, ReplyCallback const& callback);
81 void DisconnectSignal(std::string const& signal_name = "");81 void DisconnectSignal(std::string const& signal_name = "");
8282
=== modified file 'UnityCore/GnomeSessionManager.cpp'
--- UnityCore/GnomeSessionManager.cpp 2017-01-04 17:38:14 +0000
+++ UnityCore/GnomeSessionManager.cpp 2017-01-17 15:51:00 +0000
@@ -130,7 +130,7 @@
130130
131 glib::Variant tmp(g_variant_get_child_value(variant, 1), glib::StealRef());131 glib::Variant tmp(g_variant_get_child_value(variant, 1), glib::StealRef());
132 SetupLogin1Proxy(tmp.GetObjectPath());132 SetupLogin1Proxy(tmp.GetObjectPath());
133 });133 }, cancellable_);
134 }134 }
135 }135 }
136136
@@ -424,7 +424,7 @@
424424
425 if (cb)425 if (cb)
426 cb(ret, e);426 cb(ret, e);
427 });427 }, cancellable_);
428}428}
429429
430void GnomeManager::Impl::CallUPowerMethod(std::string const& method, glib::DBusProxy::ReplyCallback const& cb)430void GnomeManager::Impl::CallUPowerMethod(std::string const& method, glib::DBusProxy::ReplyCallback const& cb)
@@ -443,7 +443,7 @@
443 {443 {
444 cb(ret);444 cb(ret);
445 }445 }
446 });446 }, cancellable_);
447}447}
448448
449void GnomeManager::Impl::CallLogindMethod(std::string const& method, GVariant* parameters, glib::DBusProxy::CallFinishedCallback const& cb)449void GnomeManager::Impl::CallLogindMethod(std::string const& method, GVariant* parameters, glib::DBusProxy::CallFinishedCallback const& cb)
@@ -465,7 +465,7 @@
465 {465 {
466 cb(ret, e);466 cb(ret, e);
467 }467 }
468 });468 }, cancellable_);
469}469}
470470
471void GnomeManager::Impl::CallConsoleKitMethod(std::string const& method, GVariant* parameters)471void GnomeManager::Impl::CallConsoleKitMethod(std::string const& method, GVariant* parameters)
@@ -482,7 +482,7 @@
482 {482 {
483 LOG_ERROR(logger) << "Fallback call failed: " << e.Message();483 LOG_ERROR(logger) << "Fallback call failed: " << e.Message();
484 }484 }
485 });485 }, cancellable_);
486}486}
487487
488void GnomeManager::Impl::CallDisplayManagerSeatMethod(std::string const& method, GVariant* parameters)488void GnomeManager::Impl::CallDisplayManagerSeatMethod(std::string const& method, GVariant* parameters)
@@ -499,7 +499,7 @@
499 {499 {
500 LOG_ERROR(logger) << "DisplayManager Seat call failed: " << e.Message();500 LOG_ERROR(logger) << "DisplayManager Seat call failed: " << e.Message();
501 }501 }
502 });502 }, cancellable_);
503}503}
504504
505void GnomeManager::Impl::LockScreen(bool prompt)505void GnomeManager::Impl::LockScreen(bool prompt)
@@ -552,7 +552,7 @@
552 glib::Variant inhibitors(g_dbus_connection_call_sync(bus, test_mode_ ? testing::DBUS_NAME.c_str() : "org.gnome.SessionManager",552 glib::Variant inhibitors(g_dbus_connection_call_sync(bus, test_mode_ ? testing::DBUS_NAME.c_str() : "org.gnome.SessionManager",
553 "/org/gnome/SessionManager", "org.gnome.SessionManager",553 "/org/gnome/SessionManager", "org.gnome.SessionManager",
554 "IsInhibited", g_variant_new("(u)", Inhibited::LOGOUT), nullptr,554 "IsInhibited", g_variant_new("(u)", Inhibited::LOGOUT), nullptr,
555 G_DBUS_CALL_FLAGS_NONE, 500, nullptr, &error));555 G_DBUS_CALL_FLAGS_NONE, 500, cancellable_, &error));
556556
557 if (error)557 if (error)
558 {558 {
@@ -600,7 +600,7 @@
600 glib::Variant user_path(g_dbus_connection_call_sync(bus, "org.freedesktop.Accounts",600 glib::Variant user_path(g_dbus_connection_call_sync(bus, "org.freedesktop.Accounts",
601 "/org/freedesktop/Accounts", "org.freedesktop.Accounts",601 "/org/freedesktop/Accounts", "org.freedesktop.Accounts",
602 "FindUserByName", g_variant_new("(s)",g_get_user_name()), nullptr,602 "FindUserByName", g_variant_new("(s)",g_get_user_name()), nullptr,
603 G_DBUS_CALL_FLAGS_NONE, 500, nullptr, &error));603 G_DBUS_CALL_FLAGS_NONE, 500, cancellable_, &error));
604604
605 if (error)605 if (error)
606 {606 {
@@ -611,7 +611,7 @@
611 glib::Variant autologin(g_dbus_connection_call_sync(bus, "org.freedesktop.Accounts",611 glib::Variant autologin(g_dbus_connection_call_sync(bus, "org.freedesktop.Accounts",
612 user_path.GetObjectPath().c_str(), "org.freedesktop.DBus.Properties",612 user_path.GetObjectPath().c_str(), "org.freedesktop.DBus.Properties",
613 "Get", g_variant_new("(ss)", "org.freedesktop.Accounts.User", "AutomaticLogin"), nullptr,613 "Get", g_variant_new("(ss)", "org.freedesktop.Accounts.User", "AutomaticLogin"), nullptr,
614 G_DBUS_CALL_FLAGS_NONE, 500, nullptr, &error));614 G_DBUS_CALL_FLAGS_NONE, 500, cancellable_, &error));
615615
616 if (error)616 if (error)
617 {617 {
618618
=== modified file 'UnityCore/GnomeSessionManagerImpl.h'
--- UnityCore/GnomeSessionManagerImpl.h 2017-01-04 15:46:47 +0000
+++ UnityCore/GnomeSessionManagerImpl.h 2017-01-17 15:51:00 +0000
@@ -85,6 +85,7 @@
85 glib::DBusProxy::Ptr dm_proxy_;85 glib::DBusProxy::Ptr dm_proxy_;
86 glib::DBusProxy::Ptr dm_seat_proxy_;86 glib::DBusProxy::Ptr dm_seat_proxy_;
8787
88 glib::Cancellable cancellable_;
88 int open_sessions_;89 int open_sessions_;
89};90};
9091
9192
=== modified file 'unity-shared/SystemdWrapper.cpp'
--- unity-shared/SystemdWrapper.cpp 2016-09-01 23:50:10 +0000
+++ unity-shared/SystemdWrapper.cpp 2017-01-17 15:51:00 +0000
@@ -39,7 +39,6 @@
3939
40private:40private:
41 bool test_mode_;41 bool test_mode_;
42 glib::DBusProxy::Ptr systemd_proxy_;
43};42};
4443
45SystemdWrapper::Impl::Impl(bool test)44SystemdWrapper::Impl::Impl(bool test)