THREADING: CreateInstance critical section not protected

RESOLVED INVALID

Status

()

Core
XPCOM
P2
major
RESOLVED INVALID
18 years ago
15 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
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().

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: M6

Comment 1

18 years ago
Certainly a MP blocker. Will fix it for M6.

Updated

18 years ago
Target Milestone: M6 → M8

Updated

18 years ago
Summary: XPCOM - nsComponentManager ignores CreateInstance critical section → THREADING: CreateInstance critical section not protected

Updated

18 years ago
Target Milestone: M8 → M9

Updated

18 years ago
Target Milestone: M9 → M10

Updated

18 years ago
Target Milestone: M10 → M11

Updated

18 years ago
QA Contact: beppe → gerardok

Updated

18 years ago
Target Milestone: M11 → M13

Updated

18 years ago
Target Milestone: M13 → M14

Comment 2

18 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WONTFIX

Comment 3

17 years ago
Owner/reporter please verify, if applicable.
(Assignee)

Comment 4

17 years ago
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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Reassigning to new default XPCOM owner, hoping jband will help.

/be
Assignee: dp → scc
Status: REOPENED → NEW
QA Contact: gerardok → kandrot

Comment 6

17 years ago
Clearing out of date milestone (was M14) in hope of re-evaluation.
Target Milestone: M14 → ---

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 7

17 years ago
Edward: I'd really love for you to become the XPCOM threading guy.  Is that a
possibility?
Assignee: scc → kandrot
Status: ASSIGNED → NEW

Comment 8

17 years ago
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.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 9

16 years ago
reassign all kandrot xpcom bug.
Assignee: kandrot → dougt
Status: ASSIGNED → NEW
Target Milestone: mozilla1.0 → ---
(Assignee)

Updated

16 years ago
Target Milestone: --- → mozilla1.1
(Assignee)

Updated

16 years ago
Blocks: 101976
(Assignee)

Comment 10

15 years ago
this doesn't happen that often.  futuring.
Target Milestone: mozilla1.1alpha → Future
(Assignee)

Comment 11

15 years ago
ugh.  what I am thinking.  
Severity: normal → major
Target Milestone: Future → ---
(Assignee)

Updated

15 years ago
Target Milestone: --- → Future
(Assignee)

Comment 12

15 years ago
dp was right.  the component manager can't hold a lock when calling out to
factories.  
Status: NEW → RESOLVED
Last Resolved: 18 years ago15 years ago
Resolution: --- → INVALID
(Assignee)

Comment 13

15 years ago
*** Bug 128646 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.