Closed Bug 959484 Opened 6 years ago Closed 6 years ago
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.
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.
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.