Make ITopLevelProtocol assert to enforce its assumptions that things happen on the main thread

RESOLVED FIXED in mozilla33

Status

()

Core
IPC
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

Other Branch
mozilla33
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 8443708 [details] [diff] [review]
Make ITopLevelProtocol assert to enforce its assumptions that things happen on the main thread

Nuwa work from a few months ago made ITopLevelProtocol actors elements of linked lists, that are not guarded by any kind of lock. The assumption there is that operations that involve touching that list are done on the main thread or in a way that's somehow synchronized with it. See bug 976479 comment 94.

There's reason to believe that time would be saved if this assumption was covered by assertions. The present patch asserts IsMainThread in ITopLevelProtocol construction, destructions, and everytime we access the linked list.
Attachment #8443708 - Flags: review?(bent.mozilla)
(Assignee)

Comment 1

4 years ago
Note: this depends on bug 1028381 for NS_IsMainThread() not to return lies during child process startup.
Depends on: 1028381
Comment on attachment 8443708 [details] [diff] [review]
Make ITopLevelProtocol assert to enforce its assumptions that things happen on the main thread

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

::: ipc/glue/ProtocolUtils.h
@@ +20,5 @@
>  #include "mozilla/ipc/Shmem.h"
>  #include "mozilla/ipc/Transport.h"
>  #include "mozilla/ipc/MessageLink.h"
>  #include "mozilla/LinkedList.h"
> +#include "MainThreadUtils.h"

You should also add mozilla/Assertions.h here.
Attachment #8443708 - Flags: review?(bent.mozilla) → review+
(Assignee)

Updated

4 years ago
Depends on: 1033358
https://hg.mozilla.org/mozilla-central/rev/633893b148ec
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Depends on: 1035653
You need to log in before you can comment on or make changes to this bug.