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

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: 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.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Note: this depends on bug 1028381 for NS_IsMainThread() not to return lies during child process startup.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a54b05c9e4a1

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Backed out per bug 774388 comment 58.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d2e7bd70dd95

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

backed out again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/1370a39b329a

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/633893b148ec

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/633893b148ec