Creating instance of JS XPCOM in a thread leads to crash / deadlock

RESOLVED INVALID

Status

()

Core
XPCOM
--
critical
RESOLVED INVALID
9 years ago
9 years ago

People

(Reporter: A. Sel., Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 (.NET CLR 3.5.30729)

Creating an instance of a JS XPCOM component in a thread (that has been started from another JS XPCOM component) will lead to a crash or deadlock. Creating an instance of a C++ component WON'T lead to a crash/deadlock.

There are already two threads in the mailing list for this:
http://groups.google.com/group/mozilla.dev.tech.xpcom/browse_frm/thread/c0294500fb96ee78
http://groups.google.com/group/mozilla.dev.tech.xpcom/browse_frm/thread/12b877da4c2a53e7

I created an example extension that can be downloaded here:
http://loomo.pytalhost.com/jsxpcomcrash.zip

It adds a menu named "JSXPCOMCrash" and the two items "Crash Me" and
"Don't Crash Me".

"Crash Me" will call a method of a JS XPCOM component that starts a
new thread in which an instance of a JS XPCOM component will be
created.

"Don't Crash Me" will call a method of a JS XPCOM component that
starts a new thread in which an instance of a C++ XPCOM component will
be created.

----------------------------------------------------------------------- 

Observations so far:
- you have to restart Firefox TWICE before clicking "Crash Me" to get
a crash/dead-lock after installing this extension. pretty weird.
- the first time you click "Crash Me", there won't be a dead-lock, but
the second time.
- if you click "Crash Me" once and then "Don't Crash Me", Firefox WILL
crash! quite a surprise. 

Cheers and thanks!

Reproducible: Always

Comment 1

9 years ago
The login manager, like most XPCOM components, is only meant to be used from the main thread, and is not threadsafe at all. It doesn't help that you're also calling .createInstance on the login manager, when it is a service and you're likely to cause all sorts of problems by creating another instance of it. This bug is totally INVALID.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → INVALID
(Reporter)

Comment 2

9 years ago
I just used the nsLoginInfo as an example, as it is a simple js xpcom component that belongs to every firefox installation. In what i am working on i am creating one of my OWN js xpcom components.

Anyway, you are right, using nsLoginInfo as my personal proof I didn't connect the crash/deadlock to what it actually has something to do with: Using JS Modules in the constructor of the component. This doesn't seem to be thread-safe.

But looking at nsLoginInfo I still don't get why it is crashing, as nothing special is happening when it is created - the class is dead simple. Guess I gotta look into thread-safety a little more.

Thanks a lot. Cheers.
(Reporter)

Comment 3

9 years ago
After reading your comment again, I realized that you seem to have misread the code. I am not creating another instance of the login manager service, but nsLoginInfo - a simple JS XPCOM class for holding some DATA. You may have been confused because of the contract ID of this class: @mozilla.org/login-manager/loginInfo;1, which is similar to the login manager.

You can find nsLoginInfo in {Firefox}/components/nsLoginInfo.js. I looked at it again, and the constructor is empty, thus nothing thread-UNsafe is happening there. 

Though this is not the bug that I have in the extension that I am working on, as explained in my previous comment, there still seems to be something wrong - also evidenced by the person in one of the threads that I posted, who is simply instancing a HelloWorld class. 

Thus I think we should remove the RESOLVED INVALID status on this bug and check it again.

What do you think?

Cheers.

Comment 4

9 years ago
I think you should create a testcase that is as simple as possible and doesn't reference any external classes.
You need to log in before you can comment on or make changes to this bug.