959484 - de-THREADSAFE XPConnect

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Andrew McCreight [:mccr8]]

There are a number of vestigial THREADSAFE things in XPConnect that I think can be removed:
- NS_DECL_THREADSAFE_ISUPPORTS (change to NS_DECL_ISUPPORTS)
- NS_INTERFACE_MAP_END_THREADSAFE (change to NS_INTERFACE_MAP_END)
- nsIClassInfo::THREADSAFE (change to nsIClassInfo::MAIN_THREAD_ONLY)

With those converted, there's only one remaining instance of THREADSAFE in XPConnect:
host-4-56:mc amccreight$ grep THREADSAFE js/xpconnect/*/*.cpp js/xpconnect/*/*.h
js/xpconnect/src/XPCShellImpl.cpp:#ifdef JS_THREADSAFE

It is at least stable enough to open the browser and load a few pages.

Comment 1

[Andrew McCreight [:mccr8]]

Bug 951948 is removing some or all of this already.

Comment 2

[Andrew McCreight [:mccr8]]

https://tbpl.mozilla.org/?tree=Try&rev=5646ed8743fb

Comment 3

[Andrew McCreight [:mccr8]]

I discovered in some local testing that nsScriptError is actually used off the main thread.  This object is just a glob of strings, so it seems benign enough to leave it as threadsafe.

Thread 28 Crashed:: Compositor
nsScriptError::Release() + 188 (nsScriptError.cpp:18)
nsConsoleService::LogMessageWithMode(nsIConsoleMessage*, nsConsoleService::OutputMode) + 632 (nsConsoleService.cpp:236)
nsConsoleService::LogStringMessage(char16_t const*) + 211 (nsConsoleService.cpp:157)
mozilla::layers::CompositorOGL::Initialize() + 4829 (CompositorOGL.cpp:545)
mozilla::layers::LayerManagerComposite::Initialize() + 17 (LayerManagerComposite.cpp:120)

It looks like maybe the console service throws out a message, which is from the main thread, when something on the compositor thread pushes a new message on, or something like that.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: de-THREADSAFE XPConnect.

I don't know what that flag thing is, but hopefully MAINTHREAD is okay...

Comment 5

[Andrew McCreight [:mccr8]]

This patch is a little silly, but I think it is nice to have the code give accurate information about what is actually used on multiple threads.

Comment 6

[Bobby Holley (:bholley)]

Comment on attachment 8360685 [details] [diff] [review]
de-THREADSAFE XPConnect.

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

Yeehah! Once this lands, can we mark the dependent bug fixed?

Comment 7

[Andrew McCreight [:mccr8]]

I think so.  I can't think of anything else.

Comment 8

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ff8f88da96d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7ff8f88da96d