Ubuntu Application Launcher

Merge lp:~ted/ubuntu-app-launch/non-blocking-resume into lp:ubuntu-app-launch/touch-vivid

  1. non-blocking-resume
  2. Merge into trunk.touch-vivid
Proposed by Ted Gould
Status: Merged
Approved by: Michał Sawicz
Approved revision: 275
Merged at revision: 267
Proposed branch: lp:~ted/ubuntu-app-launch/non-blocking-resume
Merge into: lp:ubuntu-app-launch/touch-vivid
Diff against target: 470 lines (+158/-126)
6 files modified
libubuntu-app-launch/application-impl-base.cpp (+97/-75)
libubuntu-app-launch/application-impl-base.h (+15/-6)
libubuntu-app-launch/application-impl-libertine.cpp (+1/-2)
libubuntu-app-launch/registry-impl.cpp (+19/-26)
libubuntu-app-launch/snapd-info.cpp (+1/-1)
tests/libual-cpp-test.cc (+25/-16)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/non-blocking-resume
Reviewer Review Type Date Requested Status
Michał Sawicz Approve
Review via email: [email protected]

Commit message

Make resume/pause non-blocking

To post a comment you must log in.
lp:~ted/ubuntu-app-launch/non-blocking-resume updated
274. By Ted Gould

Forgot to remove emit.h from the CMakeLists.txt

Revision history for this message
Michał Sawicz (saviq) wrote :

Kaboom: http://pastebin.ubuntu.com/23455077/

review: Needs Fixing
lp:~ted/ubuntu-app-launch/non-blocking-resume updated
275. By Ted Gould

Make several functions static so that they don't depend on the object existing for their execution

Revision history for this message
Michał Sawicz (saviq) wrote :

This is the same as:

https://code.launchpad.net/~ted/ubuntu-app-launch/non-blocking-resume/+merge/310493

Just different target.

review: Approve
Revision history for this message
Pat McGowan (pat-mcgowan) wrote :

Tested silo 2181 on an MX4 running proposed and it fixed the symptoms from lp:1637996

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libubuntu-app-launch/application-impl-base.cpp'
2--- libubuntu-app-launch/application-impl-base.cpp 2016-10-03 23:54:20 +0000
3+++ libubuntu-app-launch/application-impl-base.cpp 2016-11-10 15:35:24 +0000
4@@ -257,8 +257,15 @@
5 /** Returns all the PIDs that are in the cgroup for this application */
6 std::vector<pid_t> UpstartInstance::pids()
7 {
8- auto pids = registry_->impl->pidsFromCgroup(upstartJobPath());
9- g_debug("Got %d PIDs for AppID '%s'", int(pids.size()), std::string(appId_).c_str());
10+ return pids(registry_, appId_, upstartJobPath());
11+}
12+
13+std::vector<pid_t> UpstartInstance::pids(const std::shared_ptr<Registry>& reg,
14+ const AppID& appid,
15+ const std::string& jobpath)
16+{
17+ auto pids = reg->impl->pidsFromCgroup(jobpath);
18+ g_debug("Got %d PIDs for AppID '%s'", int(pids.size()), std::string(appid).c_str());
19 return pids;
20 }
21
22@@ -267,16 +274,23 @@
23 void UpstartInstance::pause()
24 {
25 g_debug("Pausing application: %s", std::string(appId_).c_str());
26+
27+ auto registry = registry_;
28+ auto appid = appId_;
29+ auto jobpath = upstartJobPath();
30+
31+ registry->impl->thread.executeOnThread([registry, appid, jobpath] {
32+ auto pids = forAllPids(registry, appid, jobpath, [](pid_t pid) {
33+ auto oomval = oom::paused();
34+ g_debug("Pausing PID: %d (%d)", pid, int(oomval));
35+ signalToPid(pid, SIGSTOP);
36+ oomValueToPid(pid, oomval);
37+ });
38+
39+ pidListToDbus(registry, appid, pids, "ApplicationPaused");
40+ });
41+
42 registry_->impl->zgSendEvent(appId_, ZEITGEIST_ZG_LEAVE_EVENT);
43-
44- auto pids = forAllPids([this](pid_t pid) {
45- auto oomval = oom::paused();
46- g_debug("Pausing PID: %d (%d)", pid, int(oomval));
47- signalToPid(pid, SIGSTOP);
48- oomValueToPid(pid, oomval);
49- });
50-
51- pidListToDbus(pids, "ApplicationPaused");
52 }
53
54 /** Resumes this application by sending SIGCONT to all the PIDs in the
55@@ -284,16 +298,23 @@
56 void UpstartInstance::resume()
57 {
58 g_debug("Resuming application: %s", std::string(appId_).c_str());
59+
60+ auto registry = registry_;
61+ auto appid = appId_;
62+ auto jobpath = upstartJobPath();
63+
64+ registry->impl->thread.executeOnThread([registry, appid, jobpath] {
65+ auto pids = forAllPids(registry, appid, jobpath, [](pid_t pid) {
66+ auto oomval = oom::focused();
67+ g_debug("Resuming PID: %d (%d)", pid, int(oomval));
68+ signalToPid(pid, SIGCONT);
69+ oomValueToPid(pid, oomval);
70+ });
71+
72+ pidListToDbus(registry, appid, pids, "ApplicationResumed");
73+ });
74+
75 registry_->impl->zgSendEvent(appId_, ZEITGEIST_ZG_ACCESS_EVENT);
76-
77- auto pids = forAllPids([this](pid_t pid) {
78- auto oomval = oom::focused();
79- g_debug("Resuming PID: %d (%d)", pid, int(oomval));
80- signalToPid(pid, SIGCONT);
81- oomValueToPid(pid, oomval);
82- });
83-
84- pidListToDbus(pids, "ApplicationResumed");
85 }
86
87 /** Stops this instance by asking Upstart to stop it. Upstart will then
88@@ -365,7 +386,7 @@
89 */
90 void UpstartInstance::setOomAdjustment(const oom::Score score)
91 {
92- forAllPids([this, &score](pid_t pid) { oomValueToPid(pid, score); });
93+ forAllPids(registry_, appId_, upstartJobPath(), [score](pid_t pid) { oomValueToPid(pid, score); });
94 }
95
96 /** Figures out the path to the primary PID of the application and
97@@ -404,7 +425,10 @@
98
99 \param eachPid Function to run on each PID
100 */
101-std::vector<pid_t> UpstartInstance::forAllPids(std::function<void(pid_t)> eachPid)
102+std::vector<pid_t> UpstartInstance::forAllPids(const std::shared_ptr<Registry>& reg,
103+ const AppID& appid,
104+ const std::string& jobpath,
105+ std::function<void(pid_t)> eachPid)
106 {
107 std::set<pid_t> seenPids;
108 bool added = true;
109@@ -412,7 +436,7 @@
110 while (added)
111 {
112 added = false;
113- auto pidlist = pids();
114+ auto pidlist = pids(reg, appid, jobpath);
115 for (auto pid : pidlist)
116 {
117 if (seenPids.insert(pid).second)
118@@ -572,61 +596,59 @@
119 \param pids List of PIDs to turn into variants to send
120 \param signal Name of the DBus signal to send
121 */
122-void UpstartInstance::pidListToDbus(const std::vector<pid_t>& pids, const std::string& signal)
123+void UpstartInstance::pidListToDbus(const std::shared_ptr<Registry>& reg,
124+ const AppID& appid,
125+ const std::vector<pid_t>& pids,
126+ const std::string& signal)
127 {
128- auto registry = registry_;
129- auto lappid = appId_;
130-
131- registry_->impl->thread.executeOnThread([registry, lappid, pids, signal] {
132- auto vpids = std::shared_ptr<GVariant>(
133- [pids]() {
134- GVariant* pidarray = nullptr;
135-
136- if (pids.empty())
137- {
138- pidarray = g_variant_new_array(G_VARIANT_TYPE_UINT64, nullptr, 0);
139- g_variant_ref_sink(pidarray);
140- return pidarray;
141- }
142-
143- GVariantBuilder builder;
144- g_variant_builder_init(&builder, G_VARIANT_TYPE_ARRAY);
145- for (auto pid : pids)
146- {
147- g_variant_builder_add_value(&builder, g_variant_new_uint64(pid));
148- }
149-
150- pidarray = g_variant_builder_end(&builder);
151+ auto vpids = std::shared_ptr<GVariant>(
152+ [pids]() {
153+ GVariant* pidarray = nullptr;
154+
155+ if (pids.empty())
156+ {
157+ pidarray = g_variant_new_array(G_VARIANT_TYPE_UINT64, nullptr, 0);
158 g_variant_ref_sink(pidarray);
159 return pidarray;
160- }(),
161- [](GVariant* var) { g_variant_unref(var); });
162-
163- GVariantBuilder params;
164- g_variant_builder_init(&params, G_VARIANT_TYPE_TUPLE);
165- g_variant_builder_add_value(&params, g_variant_new_string(std::string(lappid).c_str()));
166- g_variant_builder_add_value(&params, vpids.get());
167-
168- GError* error = nullptr;
169- g_dbus_connection_emit_signal(registry->impl->_dbus.get(), /* bus */
170- nullptr, /* destination */
171- "/", /* path */
172- "com.canonical.UbuntuAppLaunch", /* interface */
173- signal.c_str(), /* signal */
174- g_variant_builder_end(&params), /* params, the same */
175- &error); /* error */
176-
177- if (error != nullptr)
178- {
179- g_warning("Unable to emit signal '%s' for appid '%s': %s", signal.c_str(), std::string(lappid).c_str(),
180- error->message);
181- g_error_free(error);
182- }
183- else
184- {
185- g_debug("Emmitted '%s' to DBus", signal.c_str());
186- }
187- });
188+ }
189+
190+ GVariantBuilder builder;
191+ g_variant_builder_init(&builder, G_VARIANT_TYPE_ARRAY);
192+ for (auto pid : pids)
193+ {
194+ g_variant_builder_add_value(&builder, g_variant_new_uint64(pid));
195+ }
196+
197+ pidarray = g_variant_builder_end(&builder);
198+ g_variant_ref_sink(pidarray);
199+ return pidarray;
200+ }(),
201+ [](GVariant* var) { g_variant_unref(var); });
202+
203+ GVariantBuilder params;
204+ g_variant_builder_init(&params, G_VARIANT_TYPE_TUPLE);
205+ g_variant_builder_add_value(&params, g_variant_new_string(std::string(appid).c_str()));
206+ g_variant_builder_add_value(&params, vpids.get());
207+
208+ GError* error = nullptr;
209+ g_dbus_connection_emit_signal(reg->impl->_dbus.get(), /* bus */
210+ nullptr, /* destination */
211+ "/", /* path */
212+ "com.canonical.UbuntuAppLaunch", /* interface */
213+ signal.c_str(), /* signal */
214+ g_variant_builder_end(&params), /* params, the same */
215+ &error); /* error */
216+
217+ if (error != nullptr)
218+ {
219+ g_warning("Unable to emit signal '%s' for appid '%s': %s", signal.c_str(), std::string(appid).c_str(),
220+ error->message);
221+ g_error_free(error);
222+ }
223+ else
224+ {
225+ g_debug("Emmitted '%s' to DBus", signal.c_str());
226+ }
227 }
228
229 /** Create a new Upstart Instance object that can track the job and
230
231=== modified file 'libubuntu-app-launch/application-impl-base.h'
232--- libubuntu-app-launch/application-impl-base.h 2016-09-23 22:30:51 +0000
233+++ libubuntu-app-launch/application-impl-base.h 2016-11-10 15:35:24 +0000
234@@ -108,14 +108,23 @@
235 /** A link to the registry we're using for connections */
236 std::shared_ptr<Registry> registry_;
237
238- std::vector<pid_t> forAllPids(std::function<void(pid_t)> eachPid);
239- void signalToPid(pid_t pid, int signal);
240- std::string pidToOomPath(pid_t pid);
241- void oomValueToPid(pid_t pid, const oom::Score oomvalue);
242- void oomValueToPidHelper(pid_t pid, const oom::Score oomvalue);
243- void pidListToDbus(const std::vector<pid_t>& pids, const std::string& signal);
244 std::string upstartJobPath();
245
246+ static std::vector<pid_t> forAllPids(const std::shared_ptr<Registry>& reg,
247+ const AppID& appid,
248+ const std::string& jobpath,
249+ std::function<void(pid_t)> eachPid);
250+ static std::vector<pid_t> pids(const std::shared_ptr<Registry>& reg,
251+ const AppID& appid,
252+ const std::string& jobpath);
253+ static void pidListToDbus(const std::shared_ptr<Registry>& reg,
254+ const AppID& appid,
255+ const std::vector<pid_t>& pids,
256+ const std::string& signal);
257+ static void signalToPid(pid_t pid, int signal);
258+ static void oomValueToPid(pid_t pid, const oom::Score oomvalue);
259+ static void oomValueToPidHelper(pid_t pid, const oom::Score oomvalue);
260+ static std::string pidToOomPath(pid_t pid);
261 static std::shared_ptr<gchar*> urlsToStrv(const std::vector<Application::URL>& urls);
262 static void application_start_cb(GObject* obj, GAsyncResult* res, gpointer user_data);
263 };
264
265=== modified file 'libubuntu-app-launch/application-impl-libertine.cpp'
266--- libubuntu-app-launch/application-impl-libertine.cpp 2016-10-03 23:54:20 +0000
267+++ libubuntu-app-launch/application-impl-libertine.cpp 2016-11-10 15:35:24 +0000
268@@ -255,8 +255,7 @@
269 }
270 catch (std::runtime_error& e)
271 {
272- g_debug("Unable to create application for libertine appname '%s': %s",
273- apps.get()[j], e.what());
274+ g_debug("Unable to create application for libertine appname '%s': %s", apps.get()[j], e.what());
275 }
276 }
277 }
278
279=== modified file 'libubuntu-app-launch/registry-impl.cpp'
280--- libubuntu-app-launch/registry-impl.cpp 2016-09-23 22:30:51 +0000
281+++ libubuntu-app-launch/registry-impl.cpp 2016-11-10 15:35:24 +0000
282@@ -222,17 +222,13 @@
283 if (cgManager_)
284 return;
285
286- std::promise<std::shared_ptr<GDBusConnection>> promise;
287- auto future = promise.get_future();
288-
289- thread.executeOnThread([this, &promise]() {
290+ cgManager_ = thread.executeOnThread<std::shared_ptr<GDBusConnection>>([this]() {
291 bool use_session_bus = g_getenv("UBUNTU_APP_LAUNCH_CG_MANAGER_SESSION_BUS") != nullptr;
292 if (use_session_bus)
293 {
294 /* For working dbusmock */
295 g_debug("Connecting to CG Manager on session bus");
296- promise.set_value(_dbus);
297- return;
298+ return _dbus;
299 }
300
301 auto cancel =
302@@ -241,28 +237,25 @@
303 /* Ensure that we do not wait for more than a second */
304 thread.timeoutSeconds(std::chrono::seconds{1}, [cancel]() { g_cancellable_cancel(cancel.get()); });
305
306- g_dbus_connection_new_for_address(
307- CGMANAGER_DBUS_PATH, /* cgmanager path */
308- G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, /* flags */
309- nullptr, /* Auth Observer */
310- cancel.get(), /* Cancellable */
311- [](GObject* obj, GAsyncResult* res, gpointer data) -> void {
312- GError* error = nullptr;
313- auto promise = reinterpret_cast<std::promise<std::shared_ptr<GDBusConnection>>*>(data);
314-
315- auto gcon = g_dbus_connection_new_for_address_finish(res, &error);
316- if (error != nullptr)
317- {
318- g_error_free(error);
319- }
320-
321- auto con = std::shared_ptr<GDBusConnection>(gcon, [](GDBusConnection* con) { g_clear_object(&con); });
322- promise->set_value(con);
323- },
324- &promise);
325+ GError* error = nullptr;
326+ auto retval = std::shared_ptr<GDBusConnection>(
327+ g_dbus_connection_new_for_address_sync(CGMANAGER_DBUS_PATH, /* cgmanager path */
328+ G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, /* flags */
329+ nullptr, /* Auth Observer */
330+ cancel.get(), /* Cancellable */
331+ &error),
332+ [](GDBusConnection* con) { g_clear_object(&con); });
333+
334+ if (error != nullptr)
335+ {
336+ g_warning("Unable to get CGManager connection: %s", error->message);
337+ g_error_free(error);
338+ }
339+
340+ return retval;
341 });
342
343- cgManager_ = future.get();
344+ /* NOTE: This will execute on the thread */
345 thread.timeoutSeconds(std::chrono::seconds{10}, [this]() { cgManager_.reset(); });
346 }
347
348
349=== modified file 'libubuntu-app-launch/snapd-info.cpp'
350--- libubuntu-app-launch/snapd-info.cpp 2016-10-03 23:54:08 +0000
351+++ libubuntu-app-launch/snapd-info.cpp 2016-11-10 15:35:24 +0000
352@@ -217,7 +217,7 @@
353 curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, snapd_writefunc);
354
355 /* Overridable timeout */
356- if (g_getenv("UBUNTU_APP_LAUNCH_DISABLE_SNAPD_TIMEOUT") != nullptr)
357+ if (g_getenv("UBUNTU_APP_LAUNCH_DISABLE_SNAPD_TIMEOUT") == nullptr)
358 {
359 curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, 100L);
360 }
361
362=== modified file 'tests/libual-cpp-test.cc'
363--- tests/libual-cpp-test.cc 2016-09-23 20:36:31 +0000
364+++ tests/libual-cpp-test.cc 2016-11-10 15:35:24 +0000
365@@ -1523,13 +1523,7 @@
366 }
367 };
368
369-static void signal_increment(GDBusConnection* connection,
370- const gchar* sender,
371- const gchar* path,
372- const gchar* interface,
373- const gchar* signal,
374- GVariant* params,
375- gpointer user_data)
376+static void signal_increment(const gchar* appid, GPid* pids, gpointer user_data)
377 {
378 guint* count = (guint*)user_data;
379 g_debug("Count incremented to: %d", *count + 1);
380@@ -1577,12 +1571,9 @@
381 /* Setup signal handling */
382 guint paused_count = 0;
383 guint resumed_count = 0;
384- guint paused_signal =
385- g_dbus_connection_signal_subscribe(bus, nullptr, "com.canonical.UbuntuAppLaunch", "ApplicationPaused", "/",
386- nullptr, G_DBUS_SIGNAL_FLAGS_NONE, signal_increment, &paused_count, nullptr);
387- guint resumed_signal = g_dbus_connection_signal_subscribe(
388- bus, nullptr, "com.canonical.UbuntuAppLaunch", "ApplicationResumed", "/", nullptr, G_DBUS_SIGNAL_FLAGS_NONE,
389- signal_increment, &resumed_count, nullptr);
390+
391+ ASSERT_TRUE(ubuntu_app_launch_observer_add_app_paused(signal_increment, &paused_count));
392+ ASSERT_TRUE(ubuntu_app_launch_observer_add_app_resumed(signal_increment, &resumed_count));
393
394 /* Get our app object */
395 auto appid = ubuntu::app_launch::AppID::find(registry, "com.test.good_application_1.2.3");
396@@ -1639,8 +1630,8 @@
397
398 g_spawn_command_line_sync("rm -rf " CMAKE_BINARY_DIR "/libual-proc", NULL, NULL, NULL, NULL);
399
400- g_dbus_connection_signal_unsubscribe(bus, paused_signal);
401- g_dbus_connection_signal_unsubscribe(bus, resumed_signal);
402+ ASSERT_TRUE(ubuntu_app_launch_observer_delete_app_paused(signal_increment, &paused_count));
403+ ASSERT_TRUE(ubuntu_app_launch_observer_delete_app_resumed(signal_increment, &resumed_count));
404 }
405
406 TEST_F(LibUAL, MultiPause)
407@@ -1686,10 +1677,17 @@
408 do
409 {
410 g_debug("Giving mocks a chance to start");
411- pause(200);
412+ pause(20);
413 } while (dbus_test_task_get_state(DBUS_TEST_TASK(cgmock2)) != DBUS_TEST_TASK_STATE_RUNNING &&
414 dbus_test_task_get_state(DBUS_TEST_TASK(zgmock)) != DBUS_TEST_TASK_STATE_RUNNING);
415
416+ /* Setup signal handling */
417+ guint paused_count = 0;
418+ guint resumed_count = 0;
419+
420+ ASSERT_TRUE(ubuntu_app_launch_observer_add_app_paused(signal_increment, &paused_count));
421+ ASSERT_TRUE(ubuntu_app_launch_observer_add_app_resumed(signal_increment, &resumed_count));
422+
423 /* Get our app object */
424 auto appid = ubuntu::app_launch::AppID::find(registry, "com.test.good_application_1.2.3");
425 auto app = ubuntu::app_launch::Application::create(appid, registry);
426@@ -1705,6 +1703,8 @@
427 /* Pause the app */
428 instance->pause();
429
430+ EXPECT_EVENTUALLY_EQ(1, paused_count);
431+
432 std::for_each(spews.begin(), spews.end(), [](SpewMaster& spew) { spew.reset(); });
433 pause(50);
434
435@@ -1715,6 +1715,8 @@
436 /* Now Resume the App */
437 instance->resume();
438
439+ EXPECT_EVENTUALLY_EQ(1, resumed_count);
440+
441 pause(50);
442
443 EXPECT_NE(0, std::accumulate(spews.begin(), spews.end(), int{0},
444@@ -1723,6 +1725,8 @@
445 /* Pause the app */
446 instance->pause();
447
448+ EXPECT_EVENTUALLY_EQ(2, paused_count);
449+
450 std::for_each(spews.begin(), spews.end(), [](SpewMaster& spew) { spew.reset(); });
451 pause(50);
452
453@@ -1733,12 +1737,17 @@
454 /* Now Resume the App */
455 instance->resume();
456
457+ EXPECT_EVENTUALLY_EQ(2, resumed_count);
458+
459 pause(50);
460
461 EXPECT_NE(0, std::accumulate(spews.begin(), spews.end(), int{0},
462 [](const int& acc, SpewMaster& spew) { return acc + spew.dataCnt(); }));
463
464 g_spawn_command_line_sync("rm -rf " CMAKE_BINARY_DIR "/libual-proc", NULL, NULL, NULL, NULL);
465+
466+ ASSERT_TRUE(ubuntu_app_launch_observer_delete_app_paused(signal_increment, &paused_count));
467+ ASSERT_TRUE(ubuntu_app_launch_observer_delete_app_resumed(signal_increment, &resumed_count));
468 }
469
470 TEST_F(LibUAL, OOMSet)

Subscribers

People subscribed via source and target branches