Closed Bug 959484 Opened 6 years ago Closed 6 years ago

de-THREADSAFE XPConnect

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

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.
Bug 951948 is removing some or all of this already.
Depends on: 951948
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.
I don't know what that flag thing is, but hopefully MAINTHREAD is okay...
Attachment #8360685 - Flags: review?(bobbyholley+bmo)
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 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?
Attachment #8360685 - Flags: review?(bobbyholley+bmo) → review+
I think so.  I can't think of anything else.
https://hg.mozilla.org/mozilla-central/rev/7ff8f88da96d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.