Last Comment Bug 5795 - THREADING: CreateInstance critical section not protected
: THREADING: CreateInstance critical section not protected
Status: RESOLVED INVALID
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: P2 major (vote)
: Future
Assigned To: Doug Turner (:dougt)
: Edward Kandrot
Mentors:
: 128646 (view as bug list)
Depends on:
Blocks: 101976
  Show dependency treegraph
 
Reported: 1999-04-30 13:50 PDT by Doug Turner (:dougt)
Modified: 2002-10-16 15:13 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Doug Turner (:dougt) 1999-04-30 13:50:58 PDT
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) 1999-05-01 06:51:59 PDT
Certainly a MP blocker. Will fix it for M6.
Comment 2 Suresh Duddi (gone) 2000-01-27 12:11:33 PST
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 2000-09-28 17:23:27 PDT
Owner/reporter please verify, if applicable.
Comment 4 Doug Turner (:dougt) 2000-09-28 21:42:25 PDT
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] 2000-11-10 10:26:27 PST
Reassigning to new default XPCOM owner, hoping jband will help.

/be
Comment 6 Péter Bajusz 2000-11-15 10:35:41 PST
Clearing out of date milestone (was M14) in hope of re-evaluation.
Comment 7 Scott Collins 2001-01-31 13:46:22 PST
Edward: I'd really love for you to become the XPCOM threading guy.  Is that a
possibility?
Comment 8 Edward Kandrot 2001-03-02 17:15:42 PST
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) 2001-08-28 09:18:07 PDT
reassign all kandrot xpcom bug.
Comment 10 Doug Turner (:dougt) 2002-06-04 08:39:20 PDT
this doesn't happen that often.  futuring.
Comment 11 Doug Turner (:dougt) 2002-06-04 12:51:37 PDT
ugh.  what I am thinking.  
Comment 12 Doug Turner (:dougt) 2002-10-16 14:23:10 PDT
dp was right.  the component manager can't hold a lock when calling out to
factories.  
Comment 13 Doug Turner (:dougt) 2002-10-16 15:13:42 PDT
*** Bug 128646 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.