Network Menu

Merge lp:~unity-api-team/indicator-network/lp1411714-15.04 into lp:indicator-network/15.04

  1. lp1411714-15.04
  2. Merge into trunk.15.04
Proposed by Antti Kaijanmäki
Status: Merged
Approved by: Antti Kaijanmäki
Approved revision: 472
Merged at revision: 474
Proposed branch: lp:~unity-api-team/indicator-network/lp1411714-15.04
Merge into: lp:indicator-network/15.04
Diff against target: 364 lines (+76/-78)
4 files modified
src/dbus-cpp/services/ofono.h (+10/-4)
src/indicator/modem.cpp (+53/-58)
src/menumodel-cpp/gio-helpers/util.cpp (+12/-15)
src/menumodel-cpp/gio-helpers/util.h (+1/-1)
To merge this branch: bzr merge lp:~unity-api-team/indicator-network/lp1411714-15.04
Reviewer Review Type Date Requested Status
Antti Kaijanmäki (community) Approve
Review via email: mp+248104@code.launchpad.net

Commit message

* Guard access to ofono::Modem properties that have a
  shared_ptr inside them with a mutex.
* simplify GMainLoopDispatcher

To post a comment you must log in.
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

14.09 approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/dbus-cpp/services/ofono.h'
2--- src/dbus-cpp/services/ofono.h 2014-10-23 21:46:41 +0000
3+++ src/dbus-cpp/services/ofono.h 2015-01-30 12:31:15 +0000
4@@ -1034,8 +1034,9 @@
5 break;
6 case Property::Interfaces::Type::NetworkRegistration:
7 {
8+ std::unique_lock<std::mutex> lock(_lock);
9 if (std::find(newInterfaces.begin(), newInterfaces.end(), known) == newInterfaces.end() &&
10- networkRegistration->get()) {
11+ networkRegistration.get()) {
12 networkRegistration.set(std::shared_ptr<NetworkRegistration>());
13 }
14 break;
15@@ -1046,8 +1047,9 @@
16 break;
17 case Property::Interfaces::Type::SimManager:
18 {
19+ std::unique_lock<std::mutex> lock(_lock);
20 if (std::find(newInterfaces.begin(), newInterfaces.end(), known) == newInterfaces.end() &&
21- simManager->get()) {
22+ simManager.get()) {
23 simManager.set(std::shared_ptr<SimManager>());
24 }
25 break;
26@@ -1080,7 +1082,8 @@
27 break;
28 case Property::Interfaces::Type::NetworkRegistration:
29 {
30- if (!networkRegistration->get()) {
31+ std::unique_lock<std::mutex> lock(_lock);
32+ if (!networkRegistration.get()) {
33 networkRegistration.set(std::make_shared<NetworkRegistration>(this->object));
34 }
35 break;
36@@ -1091,7 +1094,8 @@
37 break;
38 case Property::Interfaces::Type::SimManager:
39 {
40- if (!simManager->get()) {
41+ std::unique_lock<std::mutex> lock(_lock);
42+ if (!simManager.get()) {
43 simManager.set(std::make_shared<SimManager>(this->object));
44 }
45 break;
46@@ -1174,6 +1178,8 @@
47
48 std::shared_ptr<core::dbus::Signal<Signal::PropertyChanged, Signal::PropertyChanged::ArgumentType>> propertyChanged;
49
50+ // this lock must be acquired for any access to networkRegistration or simManager
51+ std::mutex _lock;
52 core::Property<NetworkRegistration::Ptr> networkRegistration;
53 core::Property<SimManager::Ptr> simManager;
54
55
56=== modified file 'src/indicator/modem.cpp'
57--- src/indicator/modem.cpp 2014-12-09 10:58:21 +0000
58+++ src/indicator/modem.cpp 2015-01-30 12:31:15 +0000
59@@ -65,20 +65,21 @@
60 });
61 });
62
63- simManagerChanged(m_ofonoModem->simManager.get());
64+ std::unique_lock<std::mutex> lock(m_ofonoModem->_lock);
65+ auto simmgr = m_ofonoModem->simManager.get();
66+ auto netreg = m_ofonoModem->networkRegistration.get();
67+ lock.unlock();
68+
69+ simManagerChanged(simmgr);
70 m_ofonoModem->simManager.changed().connect([that](org::ofono::Interface::SimManager::Ptr simmgr)
71 {
72- GMainLoopDispatch([that, simmgr](){
73- that->simManagerChanged(simmgr);
74- });
75+ that->simManagerChanged(simmgr);
76 });
77
78- networkRegistrationChanged(m_ofonoModem->networkRegistration.get());
79+ networkRegistrationChanged(netreg);
80 m_ofonoModem->networkRegistration.changed().connect([that](org::ofono::Interface::NetworkRegistration::Ptr netreg)
81 {
82- GMainLoopDispatch([that, netreg](){
83- that->networkRegistrationChanged(netreg);
84- });
85+ that->networkRegistrationChanged(netreg);
86 });
87
88 /// @todo hook up with system-settings to allow changing the identifier.
89@@ -93,14 +94,17 @@
90 } else {
91 m_simIdentifier.set(path);
92 }
93-
94 }
95
96
97 void
98 Modem::Private::update()
99 {
100+ std::unique_lock<std::mutex> lock(m_ofonoModem->_lock);
101 auto simmgr = m_ofonoModem->simManager.get();
102+ auto netreg = m_ofonoModem->networkRegistration.get();
103+ lock.unlock();
104+
105 if (simmgr) {
106 // update requiredPin
107 switch(simmgr->pinRequired.get())
108@@ -155,7 +159,6 @@
109 m_simStatus.set(SimStatus::not_available);
110 }
111
112- auto netreg = m_ofonoModem->networkRegistration.get();
113 if (netreg) {
114 m_operatorName.set(netreg->operatorName.get());
115 m_status.set(netreg->status.get());
116@@ -172,75 +175,55 @@
117 void
118 Modem::Private::networkRegistrationChanged(org::ofono::Interface::NetworkRegistration::Ptr netreg)
119 {
120+ auto that = shared_from_this();
121 if (netreg) {
122- auto that = shared_from_this();
123 netreg->operatorName.changed().connect([that](const std::string &)
124 {
125- GMainLoopDispatch([that]()
126- {
127- that->update();
128- });
129+ GMainLoopDispatch([that]() { that->update(); });
130 });
131 netreg->status.changed().connect([that](org::ofono::Interface::NetworkRegistration::Status)
132 {
133- GMainLoopDispatch([that]()
134- {
135- that->update();
136- });
137+ GMainLoopDispatch([that]() { that->update(); });
138 });
139
140 netreg->strength.changed().connect([that](std::int8_t)
141 {
142- GMainLoopDispatch([that]()
143- {
144- that->update();
145- });
146+ GMainLoopDispatch([that]() { that->update(); });
147 });
148
149 netreg->technology.changed().connect([that](org::ofono::Interface::NetworkRegistration::Technology)
150 {
151- GMainLoopDispatch([that]()
152- {
153- that->update();
154- });
155+ GMainLoopDispatch([that]() { that->update(); });
156 });
157
158 }
159- update();
160+
161+ GMainLoopDispatch([that]() { that->update(); });
162 }
163
164
165 void
166 Modem::Private::simManagerChanged(org::ofono::Interface::SimManager::Ptr simmgr)
167 {
168+ auto that = shared_from_this();
169 if (simmgr) {
170- auto that = shared_from_this();
171 simmgr->present.changed().connect([that](bool)
172 {
173- GMainLoopDispatch([that]()
174- {
175- that->update();
176- });
177+ GMainLoopDispatch([that]() { that->update(); });
178 });
179
180 simmgr->pinRequired.changed().connect([that](org::ofono::Interface::SimManager::PinType)
181 {
182- GMainLoopDispatch([that]()
183- {
184- that->update();
185- });
186+ GMainLoopDispatch([that]() { that->update(); });
187 });
188
189 simmgr->retries.changed().connect([that](std::map<org::ofono::Interface::SimManager::PinType, std::uint8_t>)
190 {
191- GMainLoopDispatch([that]()
192- {
193- that->update();
194- });
195+ GMainLoopDispatch([that]() { that->update(); });
196 });
197
198 }
199- update();
200+ GMainLoopDispatch([that]() { that->update(); });
201 }
202
203 Modem::Modem(org::ofono::Interface::Modem::Ptr ofonoModem)
204@@ -261,7 +244,11 @@
205 bool
206 Modem::enterPin(PinType type, const std::string &pin)
207 {
208- if (!d->m_ofonoModem->simManager.get()) {
209+ std::unique_lock<std::mutex> lock(d->m_ofonoModem->_lock);
210+ auto simmgr = d->m_ofonoModem->simManager.get();
211+ lock.unlock();
212+
213+ if (!simmgr) {
214 throw std::runtime_error(std::string(__PRETTY_FUNCTION__) + ": no simManager.");
215 }
216
217@@ -269,11 +256,11 @@
218 case PinType::none:
219 return true;
220 case PinType::pin:
221- return d->m_ofonoModem->simManager.get()->enterPin(org::ofono::Interface::SimManager::PinType::pin,
222- pin);
223+ return simmgr->enterPin(org::ofono::Interface::SimManager::PinType::pin,
224+ pin);
225 case PinType::puk:
226- return d->m_ofonoModem->simManager.get()->enterPin(org::ofono::Interface::SimManager::PinType::puk,
227- pin);
228+ return simmgr->enterPin(org::ofono::Interface::SimManager::PinType::puk,
229+ pin);
230 break;
231 }
232
233@@ -284,7 +271,11 @@
234 bool
235 Modem::resetPin(PinType type, const std::string &puk, const std::string &pin)
236 {
237- if (!d->m_ofonoModem->simManager.get()) {
238+ std::unique_lock<std::mutex> lock(d->m_ofonoModem->_lock);
239+ auto simmgr = d->m_ofonoModem->simManager.get();
240+ lock.unlock();
241+
242+ if (!simmgr) {
243 throw std::runtime_error(std::string(__PRETTY_FUNCTION__) + ": no simManager.");
244 }
245
246@@ -292,9 +283,9 @@
247 case PinType::none:
248 return true;
249 case PinType::puk:
250- return d->m_ofonoModem->simManager.get()->resetPin(org::ofono::Interface::SimManager::PinType::puk,
251- puk,
252- pin);
253+ return simmgr->resetPin(org::ofono::Interface::SimManager::PinType::puk,
254+ puk,
255+ pin);
256 default:
257 throw std::runtime_error(std::string(__PRETTY_FUNCTION__) + ": Not Supported.");
258 }
259@@ -303,7 +294,11 @@
260 bool
261 Modem::changePin(PinType type, const std::string &oldPin, const std::string &newPin)
262 {
263- if (!d->m_ofonoModem->simManager.get()) {
264+ std::unique_lock<std::mutex> lock(d->m_ofonoModem->_lock);
265+ auto simmgr = d->m_ofonoModem->simManager.get();
266+ lock.unlock();
267+
268+ if (!simmgr) {
269 throw std::runtime_error(std::string(__PRETTY_FUNCTION__) + ": no simManager.");
270 }
271
272@@ -311,13 +306,13 @@
273 case PinType::none:
274 return true;
275 case PinType::pin:
276- return d->m_ofonoModem->simManager.get()->changePin(org::ofono::Interface::SimManager::PinType::pin,
277- oldPin,
278- newPin);
279+ return simmgr->changePin(org::ofono::Interface::SimManager::PinType::pin,
280+ oldPin,
281+ newPin);
282 case PinType::puk:
283- return d->m_ofonoModem->simManager.get()->changePin(org::ofono::Interface::SimManager::PinType::puk,
284- oldPin,
285- newPin);
286+ return simmgr->changePin(org::ofono::Interface::SimManager::PinType::puk,
287+ oldPin,
288+ newPin);
289 break;
290 }
291
292
293=== modified file 'src/menumodel-cpp/gio-helpers/util.cpp'
294--- src/menumodel-cpp/gio-helpers/util.cpp 2014-12-09 14:56:21 +0000
295+++ src/menumodel-cpp/gio-helpers/util.cpp 2015-01-30 12:31:15 +0000
296@@ -19,43 +19,40 @@
297
298 #include "util.h"
299
300+// guards access to __funcs
301 std::mutex GMainLoopDispatch::_lock;
302-std::list<GMainLoopDispatch::Func *> GMainLoopDispatch::_funcs;
303+std::list<GMainLoopDispatch::Func> GMainLoopDispatch::__funcs;
304
305 gboolean
306 GMainLoopDispatch::dispatch_cb(gpointer)
307 {
308 std::unique_lock<std::mutex> lock(_lock);
309- for (auto func : _funcs) {
310- lock.unlock();
311- (*func)();
312- lock.lock();
313- delete func;
314+ auto funcs = __funcs;
315+ __funcs.clear();
316+ lock.unlock();
317+
318+ for (auto &func : funcs) {
319+ func();
320 }
321- _funcs.clear();
322 return G_SOURCE_REMOVE;
323 }
324
325 GMainLoopDispatch::GMainLoopDispatch(Func func, int priority, bool force_delayed)
326 {
327- std::unique_lock<std::mutex> lock(_lock);
328-
329 if (!force_delayed &&
330 g_main_context_acquire(g_main_context_default())) {
331- // already running inside GMainLoop..
332- // free the lock and dispatch immediately.
333- lock.unlock();
334+ // already running inside GMainLoop.
335 func();
336 g_main_context_release(g_main_context_default());
337 } else {
338- std::function<void()> *funcPtr = new std::function<void()>(func);
339- if (_funcs.empty()) {
340+ std::unique_lock<std::mutex> lock(_lock);
341+ if (__funcs.empty()) {
342 g_idle_add_full(priority,
343 GSourceFunc(GMainLoopDispatch::dispatch_cb),
344 NULL,
345 NULL);
346 }
347- _funcs.push_back(funcPtr);
348+ __funcs.push_back(func);
349 }
350 }
351
352
353=== modified file 'src/menumodel-cpp/gio-helpers/util.h'
354--- src/menumodel-cpp/gio-helpers/util.h 2014-12-09 14:56:21 +0000
355+++ src/menumodel-cpp/gio-helpers/util.h 2015-01-30 12:31:15 +0000
356@@ -74,7 +74,7 @@
357 typedef std::function<void()> Func;
358 static std::mutex _lock;
359 // list keeps the order of the functions
360- static std::list<Func *> _funcs;
361+ static std::list<Func> __funcs;
362
363 //GMainLoopDispatch() = delete;
364 GMainLoopDispatch(Func func, int priority=G_PRIORITY_HIGH, bool force_delayed=false);

Subscribers

People subscribed via source and target branches