Merge lp:~agateau/unity-2d/dash-fork-on-close into lp:unity-2d/3.0

Proposed by Aurélien Gâteau
Status: Rejected
Rejected by: Florian Boucault
Proposed branch: lp:~agateau/unity-2d/dash-fork-on-close
Merge into: lp:unity-2d/3.0
Diff against target: 66 lines (+34/-2)
2 files modified
places/app/dashdeclarativeview.cpp (+2/-1)
places/app/places.cpp (+32/-1)
To merge this branch: bzr merge lp:~agateau/unity-2d/dash-fork-on-close
Reviewer Review Type Date Requested Status
Florian Boucault (community) Disapprove
Review via email: mp+56792@code.launchpad.net

Description of the change

After running out of ideas to plug dash memory leaks, I came to try a workaround which can be summarized like this:

forever:
   run the dash in a subprocess
   when the user closes the view, exit the subprocess

It is definitely not a proper fix but it works well here. I believe the cost is acceptable: ld shouldn't load the libraries again so forking should remain fast, but it would need to be tested on lower-end machines.

To post a comment you must log in.
519. By Aurélien Gâteau

Do not fork endlessly when X goes down

Revision history for this message
Florian Boucault (fboucault) wrote :

Fix was committed to Qt4. This workaround is not needed anymore.

review: Disapprove

Unmerged revisions

519. By Aurélien Gâteau

Do not fork endlessly when X goes down

518. By Aurélien Gâteau

[dash] Work around memory leaks by running the application in a subprocess

See comments in places.cpp

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'places/app/dashdeclarativeview.cpp'
2--- places/app/dashdeclarativeview.cpp 2011-03-23 11:30:21 +0000
3+++ places/app/dashdeclarativeview.cpp 2011-04-07 21:46:49 +0000
4@@ -147,9 +147,10 @@
5 DashDeclarativeView::DashMode oldMode = m_mode;
6 m_mode = mode;
7 if (m_mode == HiddenMode) {
8- hide();
9 m_launcherClient->endForceVisible();
10 activeChanged(false);
11+ // See main() for why we do this instead of calling hide();
12+ QApplication::exit(12);
13 } else {
14 show();
15 raise();
16
17=== modified file 'places/app/places.cpp'
18--- places/app/places.cpp 2011-03-16 16:51:20 +0000
19+++ places/app/places.cpp 2011-04-07 21:46:49 +0000
20@@ -37,10 +37,23 @@
21 #include <unity2ddebug.h>
22 #include <unity2dtr.h>
23
24+// libc
25+#include <sys/wait.h>
26+#include <stdio.h>
27+
28 #include "dashdeclarativeview.h"
29 #include "config.h"
30
31-int main(int argc, char *argv[])
32+/*
33+ * unity-2d-places has memory leaks which seem to come from within Qt
34+ * Declarative itself. We workaround this by forking in main() and running the
35+ * whole application in a child process which stops itself after the dash
36+ * closes (see DashDeclarativeView::setDashMode()). When the child process stops,
37+ * the main process creates a new one. This way memory is released but we save
38+ * time on startup compared to the other solution of restarting the whole
39+ * application because libraries have already been loaded.
40+ */
41+int childMain(int argc, char *argv[])
42 {
43 /* gtk needs to be inited, otherwise we get an assert failure in gdk */
44 gtk_init(&argc, &argv);
45@@ -90,3 +103,21 @@
46 application.setProperty("view", QVariant::fromValue(&view));
47 return application.exec();
48 }
49+
50+int main(int argc, char* argv[])
51+{
52+ bool restart = true;
53+ while (restart) {
54+ pid_t pid = fork();
55+ if (pid == 0) {
56+ return childMain(argc, argv);
57+ } else {
58+ int status;
59+ waitpid(pid, &status, 0);
60+ fprintf(stderr, "Child exited with status %d\n", status);
61+ // If exit code is not the "restart" exit code, quit: we do not
62+ // want to prevent the session from ending
63+ restart = WEXITSTATUS(status) == 12;
64+ }
65+ }
66+}

Subscribers

People subscribed via source and target branches