Merge lp:~3v1n0/unity/ups-nhi-remove into lp:unity

Proposed by Marco Trevisan (Treviño) on 2014-11-04
Status: Merged
Approved by: Andrea Azzarone on 2014-11-10
Approved revision: 3891
Merged at revision: 3891
Proposed branch: lp:~3v1n0/unity/ups-nhi-remove
Merge into: lp:unity
Prerequisite: lp:~3v1n0/unity/ups-special-keys
Diff against target: 226 lines (+52/-64)
5 files modified
debian/control (+0/-3)
services/CMakeLists.txt (+0/-3)
services/panel-service.c (+48/-54)
tests/CMakeLists.txt (+1/-1)
unity-shared/UpstartWrapper.cpp (+3/-3)
To merge this branch: bzr merge lp:~3v1n0/unity/ups-nhi-remove
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) 2014-11-04 Approve on 2014-11-10
PS Jenkins bot (community) continuous-integration Approve on 2014-11-04
Review via email: mp+240631@code.launchpad.net

Commit message

PanelService: use gdbus to notfy upstart of service start/stop

This fixes a memory leak caused by the nih dbus proxy

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Andrea Azzarone (azzar1) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2014-09-03 07:35:09 +0000
+++ debian/control 2014-11-04 19:05:20 +0000
@@ -31,8 +31,6 @@
31 libindicator3-dev (>= 0.12.2),31 libindicator3-dev (>= 0.12.2),
32 libjson-glib-dev,32 libjson-glib-dev,
33 libjson-perl,33 libjson-perl,
34 libnih-dbus-dev,
35 libnih-dev,
36 libnotify-dev,34 libnotify-dev,
37 libnux-4.0-dev (>= 4.0.6),35 libnux-4.0-dev (>= 4.0.6),
38 libpam0g-dev,36 libpam0g-dev,
@@ -110,7 +108,6 @@
110Architecture: any108Architecture: any
111Depends: ${shlibs:Depends},109Depends: ${shlibs:Depends},
112 ${misc:Depends},110 ${misc:Depends},
113 upstart-bin,
114Description: Services for the Unity interface111Description: Services for the Unity interface
115 Unity is a desktop experience that sings. Designed by Canonical and the Ayatana112 Unity is a desktop experience that sings. Designed by Canonical and the Ayatana
116 community, Unity is all about the combination of familiarity and the future. We113 community, Unity is all about the combination of familiarity and the future. We
117114
=== modified file 'services/CMakeLists.txt'
--- services/CMakeLists.txt 2014-01-08 15:05:53 +0000
+++ services/CMakeLists.txt 2014-11-04 19:05:20 +0000
@@ -13,9 +13,6 @@
13 x1113 x11
14 libido3-0.1>=13.0.014 libido3-0.1>=13.0.0
15 libupstart15 libupstart
16 libnih
17 libnih-dbus
18 dbus-1
19)16)
2017
21pkg_check_modules(SERVICE_DEPS REQUIRED ${UNITY_PANEL_SERVICE_DEPS})18pkg_check_modules(SERVICE_DEPS REQUIRED ${UNITY_PANEL_SERVICE_DEPS})
2219
=== modified file 'services/panel-service.c'
--- services/panel-service.c 2014-11-04 19:05:20 +0000
+++ services/panel-service.c 2014-11-04 19:05:20 +0000
@@ -34,9 +34,7 @@
34#include <X11/XF86keysym.h>34#include <X11/XF86keysym.h>
35#include <X11/extensions/XInput2.h>35#include <X11/extensions/XInput2.h>
3636
37#include <upstart.h>37#include <upstart/upstart-dbus.h>
38#include <nih/alloc.h>
39#include <nih/error.h>
4038
41G_DEFINE_TYPE (PanelService, panel_service, G_TYPE_OBJECT);39G_DEFINE_TYPE (PanelService, panel_service, G_TYPE_OBJECT);
4240
@@ -87,8 +85,6 @@
8785
88 IndicatorObjectEntry *pressed_entry;86 IndicatorObjectEntry *pressed_entry;
89 gboolean use_event;87 gboolean use_event;
90
91 NihDBusProxy * upstart;
92};88};
9389
94/* Globals */90/* Globals */
@@ -139,6 +135,7 @@
139static void sort_indicators (PanelService *);135static void sort_indicators (PanelService *);
140static void notify_object (IndicatorObject *object);136static void notify_object (IndicatorObject *object);
141static void update_keybinding (GSettings *, const gchar *, gpointer);137static void update_keybinding (GSettings *, const gchar *, gpointer);
138static void emit_upstart_event (const gchar *);
142static GdkFilterReturn event_filter (GdkXEvent *, GdkEvent *, PanelService *);139static GdkFilterReturn event_filter (GdkXEvent *, GdkEvent *, PanelService *);
143140
144/*141/*
@@ -154,21 +151,8 @@
154 g_idle_remove_by_data (self);151 g_idle_remove_by_data (self);
155 gdk_window_remove_filter (NULL, (GdkFilterFunc)event_filter, self);152 gdk_window_remove_filter (NULL, (GdkFilterFunc)event_filter, self);
156153
157 if (priv->upstart != NULL && !lockscreen_mode)154 if (!lockscreen_mode)
158 {155 emit_upstart_event ("indicator-services-end");
159 int event_sent = 0;
160 event_sent = upstart_emit_event_sync (NULL, priv->upstart,
161 "indicator-services-end", NULL, 0);
162 if (event_sent != 0)
163 {
164 NihError * err = nih_error_get();
165 g_warning("Unable to signal for indicator services to stop: %s", err->message);
166 nih_free(err);
167 }
168
169 nih_unref (priv->upstart, NULL);
170 priv->upstart = NULL;
171 }
172156
173 if (GTK_IS_WIDGET (priv->last_menu) &&157 if (GTK_IS_WIDGET (priv->last_menu) &&
174 gtk_widget_get_realized (GTK_WIDGET (priv->last_menu)))158 gtk_widget_get_realized (GTK_WIDGET (priv->last_menu)))
@@ -685,17 +669,8 @@
685static gboolean669static gboolean
686ready_signal (PanelService *self)670ready_signal (PanelService *self)
687{671{
688 if (PANEL_IS_SERVICE (self) && self->priv->upstart != NULL && !lockscreen_mode)672 if (!lockscreen_mode)
689 {673 emit_upstart_event ("indicator-services-start");
690 int event_sent = 0;
691 event_sent = upstart_emit_event_sync (NULL, self->priv->upstart, "indicator-services-start", NULL, 0);
692 if (event_sent != 0)
693 {
694 NihError * err = nih_error_get();
695 g_warning("Unable to signal for indicator services to start: %s", err->message);
696 nih_free(err);
697 }
698 }
699674
700 return FALSE;675 return FALSE;
701}676}
@@ -788,6 +763,48 @@
788}763}
789764
790static void765static void
766emit_upstart_event (const gchar *event)
767{
768 const gchar *upstartsession = g_getenv ("UPSTART_SESSION");
769
770 if (!upstartsession)
771 return;
772
773 GError *error = NULL;
774 GDBusConnection* conn = g_dbus_connection_new_for_address_sync (upstartsession,
775 G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT,
776 NULL, NULL, &error);
777
778 if (error)
779 {
780 g_warning ("Unable to connect to Upstart session: %s", error->message);
781 g_error_free (error);
782 return;
783 }
784
785 GVariant *result = g_dbus_connection_call_sync (conn, DBUS_SERVICE_UPSTART,
786 DBUS_PATH_UPSTART,
787 DBUS_INTERFACE_UPSTART,
788 "EmitEvent",
789 g_variant_new ("(sasb)", event, NULL, 0),
790 NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START, -1,
791 NULL, &error);
792
793 if (error)
794 {
795 g_warning ("Unable to emit Upstart event: %s", error->message);
796 g_error_free (error);
797 }
798 else
799 {
800 g_variant_unref (result);
801 }
802
803 g_object_unref (conn);
804}
805
806
807static void
791panel_service_init (PanelService *self)808panel_service_init (PanelService *self)
792{809{
793 PanelServicePrivate *priv;810 PanelServicePrivate *priv;
@@ -811,29 +828,6 @@
811 update_keybinding (priv->gsettings, MENU_TOGGLE_KEYBINDING_KEY, &priv->menu_toggle);828 update_keybinding (priv->gsettings, MENU_TOGGLE_KEYBINDING_KEY, &priv->menu_toggle);
812 update_keybinding (priv->gsettings, SHOW_DASH_KEY, &priv->show_dash);829 update_keybinding (priv->gsettings, SHOW_DASH_KEY, &priv->show_dash);
813 update_keybinding (priv->gsettings, SHOW_HUD_KEY, &priv->show_hud);830 update_keybinding (priv->gsettings, SHOW_HUD_KEY, &priv->show_hud);
814
815 const gchar *upstartsession = g_getenv ("UPSTART_SESSION");
816 if (upstartsession != NULL && !lockscreen_mode)
817 {
818 DBusConnection *conn = dbus_connection_open (upstartsession, NULL);
819 if (conn != NULL)
820 {
821 priv->upstart = nih_dbus_proxy_new (NULL, conn,
822 NULL,
823 DBUS_PATH_UPSTART,
824 NULL, NULL);
825 if (priv->upstart == NULL)
826 {
827 NihError * err = nih_error_get();
828 g_warning("Unable to get Upstart proxy: %s", err->message);
829 nih_free(err);
830 }
831 dbus_connection_unref (conn);
832 }
833 }
834
835 if (priv->upstart != NULL)
836 priv->upstart->auto_start = FALSE;
837}831}
838832
839static gboolean833static gboolean
840834
=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt 2014-10-14 22:37:37 +0000
+++ tests/CMakeLists.txt 2014-11-04 19:05:20 +0000
@@ -8,7 +8,7 @@
8#8#
9# Unit tests9# Unit tests
10#10#
11set (TEST_DEPS "${UNITY_PLUGIN_DEPS};unity>=4.99.0;libupstart;libnih;libnih-dbus;dbus-1")11set (TEST_DEPS "${UNITY_PLUGIN_DEPS};unity>=4.99.0;libupstart;")
12pkg_check_modules (TEST_UNIT_DEPS REQUIRED ${TEST_DEPS})12pkg_check_modules (TEST_UNIT_DEPS REQUIRED ${TEST_DEPS})
1313
14string (REPLACE ";" " " TEST_UNIT_DEPS_CFLAGS_OTHER "${TEST_UNIT_CFLAGS_OTHER}")14string (REPLACE ";" " " TEST_UNIT_DEPS_CFLAGS_OTHER "${TEST_UNIT_CFLAGS_OTHER}")
1515
=== modified file 'unity-shared/UpstartWrapper.cpp'
--- unity-shared/UpstartWrapper.cpp 2014-03-04 14:10:26 +0000
+++ unity-shared/UpstartWrapper.cpp 2014-11-04 19:05:20 +0000
@@ -20,6 +20,7 @@
20#include "UpstartWrapper.h"20#include "UpstartWrapper.h"
2121
22#include <UnityCore/GLibDBusProxy.h>22#include <UnityCore/GLibDBusProxy.h>
23#include <upstart/upstart-dbus.h>
2324
24namespace unity25namespace unity
25{26{
@@ -41,9 +42,8 @@
4142
42UpstartWrapper::Impl::Impl(bool test_mode)43UpstartWrapper::Impl::Impl(bool test_mode)
43{44{
44 upstart_proxy_ = std::make_shared<unity::glib::DBusProxy>(test_mode ? "com.canonical.Unity.Test.Upstart" : "com.ubuntu.Upstart",45 upstart_proxy_ = std::make_shared<unity::glib::DBusProxy>(test_mode ? "com.canonical.Unity.Test.Upstart" : DBUS_SERVICE_UPSTART,
45 "/com/ubuntu/Upstart", 46 DBUS_PATH_UPSTART, DBUS_INTERFACE_UPSTART);
46 "com.ubuntu.Upstart0_6");
47}47}
4848
49void UpstartWrapper::Impl::Emit(std::string const& name)49void UpstartWrapper::Impl::Emit(std::string const& name)