5795 - THREADING: CreateInstance critical section not protected

Status: RESOLVED INVALID

(Core :: XPCOM, defect, P2)

Description

[Doug Turner (:dougt)]

nsComponentManager ignores CreateInstance critical section.  nsComponentManager
should place a monitor before and after the call to a components
CreateInstance() meathod to avoid memory leaks or worse.

An example of this is a singleton component.  I may have code like this:

if (mInstance == NULL)
{
        mInstance = new nsProxyEventManager();
}
return mInstance;


If two or more threads are in a step lock state, mInstance will be allocated
more than once.

To avoid this, I would suggest the use of monitors similar to what is done with
RegisterFactory().

Comment 1

[Suresh Duddi (gone)]

Certainly a MP blocker. Will fix it for M6.

Comment 2

[Suresh Duddi (gone)]

The more I think about this, the more I dont want to do this. This would mean
that componentmanager would hold a lock before calling out to create any
instance in the module and release the lock after getting back control. What if
the module turns around and calls cm for other things that would cause the same
component to be created again. That would cause a deadlock.

Any module holding a lock while calling out should be ultra cautious.

I dont see a problem unless in the singleton case. I think I would be
comfortable in saying the singleton should protect against threads just as it
would in any multi threaded environment. Of course, any singleton mechanism we
provide will take care of this.

So I vote to WONTFIX this bug. Opinions. We can discuss this further in
n.p.m.xpcom if you disagree.

Comment 3

[Jonas Nagel]

Owner/reporter please verify, if applicable.

Comment 4

[Doug Turner (:dougt)]

I am going to reopen.  dp.  we need to have a thread save version of the
component manager.  I believe that there is another bug like this.

Comment 5

[Brendan Eich [:brendan]]

Reassigning to new default XPCOM owner, hoping jband will help.

/be

Comment 6

[Péter Bajusz]

Clearing out of date milestone (was M14) in hope of re-evaluation.

Comment 7

[Scott Collins]

Edward: I'd really love for you to become the XPCOM threading guy.  Is that a
possibility?

Comment 8

[Edward Kandrot]

I will look into this and see what I can find out.  I am not currently savy on
the issues involved, so I will start with learning why it is needed and what the
impact would be from doing this.  I do not like needless locks, since it can
kill a program, so I want to know all about the issues before I make any changes
in this area.

Comment 9

[Doug Turner (:dougt)]

reassign all kandrot xpcom bug.

Comment 10

[Doug Turner (:dougt)]

this doesn't happen that often.  futuring.

Comment 11

[Doug Turner (:dougt)]

ugh.  what I am thinking.  

Comment 12

[Doug Turner (:dougt)]

dp was right.  the component manager can't hold a lock when calling out to
factories.  

Comment 13

[Doug Turner (:dougt)]

*** Bug 128646 has been marked as a duplicate of this bug. ***