Code review comment for lp:~thomas-voss/location-service/fix-nm-race

Revision history for this message
Loïc Minier (lool) wrote :

Hmm the locking is a bit spaghetti now; I can see why lock needs to be passed around to unlock before dispatching the updates from the on_foo() callbacks, but it does feel a bit weird to pass the lock down a callback and unlock it from there.

I guess the only way would be to queue the added/removed wifis/cells into a queue and trigger the updates once the lock is released, but that's pretty much as weird.

Anyway, tested, and seems to help with the otherwise failing test case.

Note that: a) I wasn't able to reproduce on mako + vivid, so couldn't confirm this fixed anything; b) it did help reproducible breakage on krillin from earlier revisions, but I did manage to break HERE on one boot.

So generally, this helps; I suspect there's still a race for a case.

I did witness tech=cell switching to tech=wifi on krillin when coming out of airplane mode, which was nice!

review: Approve

« Back to merge proposal