646398 - Potential lock order inconsistency (potential deadlock) in nsCacheService.cpp

Status: RESOLVED DUPLICATE of bug 646259

(Core :: Networking: Cache, defect)

Description

[Julian Seward [:jseward]]

It looks like there's a lock order inconsistency (possible deadlock?)
in netwerk/cache/nsCacheService.cpp.

--- Analysis ---------------------------------------------------

The two locks involved are PRLocks, hence

   nsCacheService::mLock    

and the PRLock buried inside

   nsCacheService::mMonitor


The conflicting orderings appear in the following methods:

nsCacheService::SyncWithCacheIOThread()   (nsCacheService.cpp)

829    // wait until notified, then return
830    rv = autoMonitor.Wait();
831    Lock();

After 830, the thread holds the lock in mMonitor (since there's a
pthread_cond_wait at that point, so the thread atomically
(re-)acquires the associated lock.)  It then goes on at 831 to acquire
mLock.


vs


nsCacheService::OnProfileShutdown(PRBool cleanse)   (nsCacheService.cpp)

First it acquires mLock, then calls
nsCacheService::SyncWithCacheIOThread() which acquires the
mMonitor lock.

1964    nsCacheServiceAutoLock lock;
....
1971    (void) SyncWithCacheIOThread();
....
810  nsCacheService::SyncWithCacheIOThread()
811  {
...
817      mozilla::MonitorAutoEnter autoMonitor(gService->mMonitor);




--- Original bug report from Helgrind --------------------------

Thread #1: lock order "0x207ABE00 before 0x207ABCF8" violated

Observed (incorrect) order is: acquisition of lock at 0x207ABCF8
   at 0x4C27A60: pthread_cond_wait_WRK (/home/sewardj/VgTRUNK/HGDEV2/helgrind/hg_intercepts.c:685)
   by 0x4C28A60: pthread_cond_wait@* (/home/sewardj/VgTRUNK/HGDEV2/helgrind/hg_intercepts.c:707)
   by 0x799645C: PR_WaitCondVar (nsprpub/pr/src/pthreads/ptsynch.c:433)
   by 0x79964C1: PR_Wait (nsprpub/pr/src/pthreads/ptsynch.c:647)
   by 0x569879D: mozilla::Monitor::Wait(unsigned int) (ff-opt/netwerk/cache/../../dist/include/mozilla/Monitor.h:123)
   by 0x56987BB: mozilla::MonitorAutoEnter::Wait(unsigned int) (ff-opt/ipc/glue/../../dist/include/mozilla/Monitor.h:226)
   by 0x5695B3A: nsCacheService::SyncWithCacheIOThread() (netwerk/cache/nsCacheService.cpp:830)
   by 0x56964D5: nsCacheService::OnProfileShutdown(int) (netwerk/cache/nsCacheService.cpp:1971)
   by 0x5697EDA: nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) (netwerk/cache/nsCacheService.cpp:389)
   by 0x6303AFC: nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverList.cpp:130)
   by 0x6303BA8: nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverService.cpp:182)
   by 0x562951F: nsXREDirProvider::DoShutdown() (toolkit/xre/nsXREDirProvider.cpp:808)

 followed by a later acquisition of lock at 0x207ABE00
   at 0x4C2853D: pthread_mutex_lock (/home/sewardj/VgTRUNK/HGDEV2/helgrind/hg_intercepts.c:496)
   by 0x79965A5: PR_Lock (nsprpub/pr/src/pthreads/ptsynch.c:208)
   by 0x56944C7: nsCacheService::Lock() (netwerk/cache/nsCacheService.cpp:2214)
   by 0x5695B41: nsCacheService::SyncWithCacheIOThread() (netwerk/cache/nsCacheService.cpp:831)
   by 0x56964D5: nsCacheService::OnProfileShutdown(int) (netwerk/cache/nsCacheService.cpp:1971)
   by 0x5697EDA: nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) (netwerk/cache/nsCacheService.cpp:389)
   by 0x6303AFC: nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverList.cpp:130)
   by 0x6303BA8: nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverService.cpp:182)
   by 0x562951F: nsXREDirProvider::DoShutdown() (toolkit/xre/nsXREDirProvider.cpp:808)
   by 0x5622E9B: ScopedXPCOMStartup::~ScopedXPCOMStartup() (toolkit/xre/nsAppRunner.cpp:1110)
   by 0x56269D7: XRE_main (toolkit/xre/nsAppRunner.cpp:3794)
   by 0x400E3E: main (browser/app/nsBrowserApp.cpp:158)

Required order was established by acquisition of lock at 0x207ABE00
   at 0x4C2853D: pthread_mutex_lock (/home/sewardj/VgTRUNK/HGDEV2/helgrind/hg_intercepts.c:496)
   by 0x79965A5: PR_Lock (nsprpub/pr/src/pthreads/ptsynch.c:208)
   by 0x56944C7: nsCacheService::Lock() (netwerk/cache/nsCacheService.cpp:2214)
   by 0x5693E48: nsCacheServiceAutoLock::nsCacheServiceAutoLock() (netwerk/cache/nsCacheService.h:305)
   by 0x56964B8: nsCacheService::OnProfileShutdown(int) (netwerk/cache/nsCacheService.cpp:1964)
   by 0x5697EDA: nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) (netwerk/cache/nsCacheService.cpp:389)
   by 0x6303AFC: nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverList.cpp:130)
   by 0x6303BA8: nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverService.cpp:182)
   by 0x562951F: nsXREDirProvider::DoShutdown() (toolkit/xre/nsXREDirProvider.cpp:808)
   by 0x5622E9B: ScopedXPCOMStartup::~ScopedXPCOMStartup() (toolkit/xre/nsAppRunner.cpp:1110)
   by 0x56269D7: XRE_main (toolkit/xre/nsAppRunner.cpp:3794)
   by 0x400E3E: main (browser/app/nsBrowserApp.cpp:158)

 followed by a later acquisition of lock at 0x207ABCF8
   at 0x4C2853D: pthread_mutex_lock (/home/sewardj/VgTRUNK/HGDEV2/helgrind/hg_intercepts.c:496)
   by 0x79965A5: PR_Lock (nsprpub/pr/src/pthreads/ptsynch.c:208)
   by 0x79966BD: PR_EnterMonitor (nsprpub/pr/src/pthreads/ptsynch.c:594)
   by 0x5698757: mozilla::Monitor::Enter() (ff-opt/netwerk/cache/../../dist/include/mozilla/Monitor.h:104)
   by 0x569876C: mozilla::MonitorAutoEnter::MonitorAutoEnter(mozilla::Monitor&) (ff-opt/netwerk/cache/../../dist/include/mozilla/Monitor.h:216)
   by 0x5695AD9: nsCacheService::SyncWithCacheIOThread() (netwerk/cache/nsCacheService.cpp:816)
   by 0x56964D5: nsCacheService::OnProfileShutdown(int) (netwerk/cache/nsCacheService.cpp:1971)
   by 0x5697EDA: nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) (netwerk/cache/nsCacheService.cpp:389)
   by 0x6303AFC: nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverList.cpp:130)
   by 0x6303BA8: nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (xpcom/ds/nsObserverService.cpp:182)
   by 0x562951F: nsXREDirProvider::DoShutdown() (toolkit/xre/nsXREDirProvider.cpp:808)
   by 0x5622E9B: ScopedXPCOMStartup::~ScopedXPCOMStartup() (toolkit/xre/nsAppRunner.cpp:1110)

Comment 1

[Josh Matthews [:jdm]]

Do you still see this with the patch from bug 646259 applied?

Comment 2

[Julian Seward [:jseward]]

(In reply to comment #1)
I'd say it's likely the same bug, but I'm having trouble applying
the patch for bug 646259 comment 2 to a M-C checkout of 24 March.
Looks like there have been some other changes to nsCacheService.{cpp,h}
that cause the patch not to apply.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think you need to apply the patches in Bug 645263 first.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Thanks Julian, good to see that helgrind and our internal tool agree :).

Bug 645263 and bug 646259 will be landing together, hopefully this week.