930952 - [DBus] DBus message are not dispatched immediately

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Received DBus messages might get delayed indefinitely before being dispatched.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 930952: Dispatch DBus messages after reading socket

Hi Kyle,

I noticed the following problem while refactoring the DBus code. Could you take a look at this?

After reading DBus messages with dbus_watch_handle, there is a break statement, and the DBus messages are only dispatched after we read from the control socket, which might not happen until much later. The patch fixed this problem.

There is a comment next to the break, so the current code might be correct, but to me it looks like the break should be removed at all, or maybe moved into the control-socket branch.

Thanks!

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 822249 [details] [diff] [review]
[01] Bug 930952: Dispatch DBus messages after reading socket

Review of attachment 822249 [details] [diff] [review]:
-----------------------------------------------------------------

So I agree with the idea that we should dispatch before breaking, but the solution is this patch has us ONLY dispatching when there's an event watch called. Unless I'm missing something about how watches work, I think instead we want to set a status flag in the else block, fall out of the else block, leave the dispatch in the same place it was before, then check status we set in else block and break if true there. That means we'll always dispatch after receiving any poll event, not just watches.

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

As far as I understand the API, to process messages we first have to read from the watch by calling dbus_watch_handle, and afterwards dispatch messages one by one. We never call dbus_watch_handle in the if branch, so there is no point in running the dispatch loop. Am I missing something here?

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ah, yeah, you're right. Been long enough since I'd been in the dbus stuff myself that I forgot about the specifics of it. Flipping my fb :)

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 822249 [details] [diff] [review]
[01] Bug 930952: Dispatch DBus messages after reading socket

Ok, thanks. Can you review it?

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thank you.

https://hg.mozilla.org/integration/b2g-inbound/rev/568f78e76079

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=568f78e76079

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/568f78e76079