service manager assertions when starting viewer

RESOLVED WORKSFORME

Status

()

Core
XPCOM
P3
normal
RESOLVED WORKSFORME
18 years ago
15 years ago

People

(Reporter: dbaron, Assigned: dougt)

Tracking

Trunk
x86
Windows 98
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

18 years ago
There are three assertions about reentering the service manager when starting 
viewer on Win98.  This is a bit of a pain when running regression tests.  This 
is the assertion we added a few days ago after fixing the only place it happened 
in mozilla.  We should figure out what's happening here too.

Comment 1

18 years ago
Hrm, I'm not seeing these on NT. Could you grab stack traces?
Status: NEW → ASSIGNED
(Reporter)

Comment 2

18 years ago
Here are the stack traces for the three assertions:

KERNEL32! bff768a0()
nsDebug::Assertion(const char * 0x0053d10c, const char * 0x0053d0f4, const char 
* 0x0053d0b4, int 355) line 246 + 13 bytes
nsServiceManagerImpl::GetService(nsServiceManagerImpl * const 0x00be4d30, const 
nsID & {...}, const nsID & {...}, nsISupports * * 0x0203fc6c, 
nsIShutdownListener * 0x00000000) line 355 + 44 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports 
* * 0x0203fc6c, nsIShutdownListener * 0x00000000) line 562
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x0203fc6c) line 46 + 
22 bytes
nsCOMPtr<nsIMIMEService>::assign_from_helper(const nsCOMPtr_helper & {...}, 
const nsID & {...}) line 856 + 18 bytes
nsCOMPtr<nsIMIMEService>::nsCOMPtr<nsIMIMEService>(const nsCOMPtr_helper & 
{...}) line 553
nsFileIO::Open(nsFileIO * const 0x00cd0890, char * * 0x00cd0900, int * 
0x00cd0928) line 156 + 30 bytes
nsFileTransport::Process() line 478 + 58 bytes
nsFileTransport::Run(nsFileTransport * const 0x00cd08e4) line 452
nsThreadPoolRunnable::Run(nsThreadPoolRunnable * const 0x0132bb50) line 689 + 12 
bytes
nsThread::Main(void * 0x0132bb00) line 84 + 26 bytes
_PR_NativeRunThread(void * 0x0132b990) line 399 + 13 bytes
_threadstartex(void * 0x0132b7e0) line 212 + 13 bytes
KERNEL32! bff88f20()
KERNEL32! bff869ef()
KERNEL32! bff868ec()

KERNEL32! bff768a0()
nsDebug::Assertion(const char * 0x0053d10c, const char * 0x0053d0f4, const char 
* 0x0053d0b4, int 355) line 246 + 13 bytes
nsServiceManagerImpl::GetService(nsServiceManagerImpl * const 0x00be4d30, const 
nsID & {...}, const nsID & {...}, nsISupports * * 0x01f1fc6c, 
nsIShutdownListener * 0x00000000) line 355 + 44 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports 
* * 0x01f1fc6c, nsIShutdownListener * 0x00000000) line 562
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x01f1fc6c) line 46 + 
22 bytes
nsCOMPtr<nsIMIMEService>::assign_from_helper(const nsCOMPtr_helper & {...}, 
const nsID & {...}) line 856 + 18 bytes
nsCOMPtr<nsIMIMEService>::nsCOMPtr<nsIMIMEService>(const nsCOMPtr_helper & 
{...}) line 553
nsFileIO::Open(nsFileIO * const 0x01326050, char * * 0x013260c0, int * 
0x013260e8) line 156 + 30 bytes
nsFileTransport::Process() line 478 + 58 bytes
nsFileTransport::Run(nsFileTransport * const 0x013260a4) line 452
nsThreadPoolRunnable::Run(nsThreadPoolRunnable * const 0x00cd3410) line 689 + 12 
bytes
nsThread::Main(void * 0x00cd4410) line 84 + 26 bytes
_PR_NativeRunThread(void * 0x01328eb0) line 399 + 13 bytes
_threadstartex(void * 0x01328d00) line 212 + 13 bytes
KERNEL32! bff88f20()
KERNEL32! bff869ef()
KERNEL32! bff868ec()


KERNEL32! bff768a0()
nsDebug::Assertion(const char * 0x0053d10c, const char * 0x0053d0f4, const char 
* 0x0053d0b4, int 355) line 246 + 13 bytes
nsServiceManagerImpl::GetService(nsServiceManagerImpl * const 0x00be4d30, const 
nsID & {...}, const nsID & {...}, nsISupports * * 0x01dffc6c, 
nsIShutdownListener * 0x00000000) line 355 + 44 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports 
* * 0x01dffc6c, nsIShutdownListener * 0x00000000) line 562
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x01dffc6c) line 46 + 
22 bytes
nsCOMPtr<nsIMIMEService>::assign_from_helper(const nsCOMPtr_helper & {...}, 
const nsID & {...}) line 856 + 18 bytes
nsCOMPtr<nsIMIMEService>::nsCOMPtr<nsIMIMEService>(const nsCOMPtr_helper & 
{...}) line 553
nsFileIO::Open(nsFileIO * const 0x01325900, char * * 0x01325b20, int * 
0x01325b48) line 156 + 30 bytes
nsFileTransport::Process() line 478 + 58 bytes
nsFileTransport::Run(nsFileTransport * const 0x01325b04) line 452
nsThreadPoolRunnable::Run(nsThreadPoolRunnable * const 0x01327c80) line 689 + 12 
bytes
nsThread::Main(void * 0x01327c30) line 84 + 26 bytes
_PR_NativeRunThread(void * 0x01327ac0) line 399 + 13 bytes
_threadstartex(void * 0x01327910) line 212 + 13 bytes
KERNEL32! bff88f20()
KERNEL32! bff869ef()
KERNEL32! bff868ec()

I was not able to break on the inner GetService.  Could this have something to 
do with getting the service on multiple threads at the same time?
(Reporter)

Comment 3

18 years ago
Leak log from leaksoup:



<void*> (16) Leaks



0x0A34E138 [0] <void*> (16){19946}

        0x0B26401C      ·&@·

        0x0A372258 [1] <nsMIMEService> (16){19930}

        0x00000000

        0x00000000

malloc

operator new(unsigned long, const std::nothrow_t&)

operator new(unsigned long)

nsServiceManagerImpl::GetService(const nsID&, const nsID&, nsISupports**, 

nsIShutdownListener*)

nsServiceManager::GetService(const nsID&, const nsID&, nsISupports**, 

nsIShutdownListener*)

nsGetServiceByCID::operator()(const nsID&, void**) const

nsCOMPtr<14nsIMIMEService>::assign_from_helper(const nsCOMPtr_helper&, const 

nsID&)

nsFileIO::Open(char**, int*)

nsFileTransport::Process()

nsFileTransport::Run()

nsThreadPoolRunnable::Run()

nsThread::Main(void*)

_PR_UserRunThread

_PR_UserRunThread

Comment 4

18 years ago
We tracked at least one of these assertions down to a race condition that occurs
when the service manager's monitor is exited, here:

http://lxr.mozilla.org/mozilla/source/xpcom/components/nsServiceManager.cpp#346

We tried taking out the monitor exit (so that the service manager holds the
monitor through the CreateInstance call), and ended up triggering a bunch of
brendan-assertions that complain about dangerous lock ordering.

This is definitely bad, but nobody had great ideas about how to fix.

Comment 5

18 years ago
brendan suggested putting dummy record into the hashtable before we call out to
CreateInstance(). Then, a nested call to GetService() would discover the dummy
record and could return NS_ERROR_REGISTER_AGAIN_LATER or something.

Of course, this doesn't really *solve* the problem of the nested service, but at
least it provides a better way to detect the problem and recover.

Adding rayw and scc to the cc list: any chance I could convince either of you to
hack this out?
In threaded systems, racing cache fills often must rendezvous via a "fill in 
progress" dummy (really, incomplete) entry.  We could use nsServiceEntry with a 
null mService to signify such a state.

The problem here of a nested GetService, where there is no concurrency and there 
is presumably a data dependency on the return value of the inner GetService, 
cannot be solved with a new state and error result alone, although as waterson 
says, it beats an assertbotch!  What we should endeavor to do is un-nest the 
control flow, or otherwise undo the data dependency.

Maybe we can use the new error result to cause a deferral through the event 
loop.  That's a dignified old hack that has saved the day numerous times in the 
MozillaClassic codebase.

/be

Comment 7

17 years ago
WFM win98 I think the asserts stopped 10 days ago.
(Reporter)

Comment 8

17 years ago
Even if the assertions stopped, the underlying thread safety problems probably
still exist.

Comment 9

17 years ago
Giving to the XPCOM owner.
Assignee: waterson → scc
Status: ASSIGNED → NEW
Component: Viewer App → XPCOM
QA Contact: rickg → kandrot

Comment 10

15 years ago
Giving to the XPCOM owner.
Assignee: scc → dougt
QA Contact: kandrot → scc
(Assignee)

Comment 11

15 years ago
we already have thread safety bugs which address most of the thread problems in
XPCOM.  I want to close this bug since it really doesn't specifically address
any of the real problems.  (Check out my bug list for the real xpcom
threadsafety problems.)  i am marking wfm but it really may just be a dup.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 12

15 years ago
These assertions, were, IIRC, a sign that the service manager was creating
*multiple instances* of a service.  That's bad.  What other bug covers this problem?
(Assignee)

Comment 13

15 years ago
I think that http://bugzilla.mozilla.org/show_bug.cgi?id=5795 would cover this
problem.  Note that it is marked INVALID.  Maybe we should be locking in the
generic factory code... ick.  I would rather not.  

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