Closed Bug 1028383 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: IPC, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

Attachments

(1 file)

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)
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+
Depends on: 1033358
https://hg.mozilla.org/mozilla-central/rev/633893b148ec
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.