Closed Bug 646259 Opened 13 years ago Closed 13 years ago

"###!!! ASSERTION: Potential deadlock detected" in nsCacheService

Categories

(Core :: Networking: Cache, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: cjones, Assigned: cjones)

References

Details

Attachments

(3 files)

Attached file Output from test
The patches in bug 645263 have the cache service participate in deadlock detection.  The cache service has an invalid lock ordering, apparently somewhere around shutdown.  Relevant output is 

$ make SOLO_FILE="test_clearHistory_shutdown.js" -C ../ff-dbg/browser/components/places/tests/ check-one
[snip]
WARNING: Resource acquired at calling context
  mozilla::Mutex::Lock() at BlockingResourceBase.cpp:258
  nsCacheService::Lock() at nsCacheService.cpp:2212
  nsCacheServiceAutoLock at nsCacheService.h:306
  nsCacheService::EvictEntriesForClient(char const*, int) at nsCacheService.cpp:1195
  nsCacheService::EvictEntries(int) at nsCacheService.cpp:1326
[snip]
is being released in non-LIFO order; why?

###!!! ERROR: Potential deadlock detected:
=== Cyclical dependency starts at
--- Mutex : nsCacheService.mLock calling context
  mozilla::Mutex::Lock() at BlockingResourceBase.cpp:258
  nsCacheService::Lock() at nsCacheService.cpp:2212
  nsCacheServiceAutoLock at nsCacheService.h:306
  nsCacheService::SetDiskCacheCapacity(int) at nsCacheService.cpp:2073
  nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) at nsCacheService.cpp:429
[snip]
--- Next dependency:
--- Monitor : nsCacheService.mMonitor (currently acquired)
 calling context
  mozilla::Monitor::Enter() at BlockingResourceBase.cpp:290
  MonitorAutoEnter at Monitor.h:217
  nsCacheService::SyncWithCacheIOThread() at nsCacheService.cpp:818
  nsDiskCacheDevice::Shutdown_Private(int) at nsDiskCacheDevice.cpp:434
  nsDiskCacheDevice::ClearDiskCache() at nsDiskCacheDevice.cpp:966
  nsDiskCacheDevice::EvictEntries(char const*) at nsDiskCacheDevice.cpp:888
  nsCacheService::EvictEntriesForClient(char const*, int) at nsCacheService.cpp:1206
  nsCacheService::EvictEntries(int) at nsCacheService.cpp:1326
[snip]
=== Cycle completed at
--- Mutex : nsCacheService.mLock calling context
  mozilla::Mutex::Lock() at BlockingResourceBase.cpp:258
  nsCacheService::Lock() at nsCacheService.cpp:2212
  nsCacheService::SyncWithCacheIOThread() at nsCacheService.cpp:835
  nsDiskCacheDevice::Shutdown_Private(int) at nsDiskCacheDevice.cpp:434
  nsDiskCacheDevice::ClearDiskCache() at nsDiskCacheDevice.cpp:966
  nsDiskCacheDevice::EvictEntries(char const*) at nsDiskCacheDevice.cpp:888
  nsCacheService::EvictEntriesForClient(char const*, int) at nsCacheService.cpp:1206
  nsCacheService::EvictEntries(int) at nsCacheService.cpp:1326
[snip]

Full output is attached.  Not sure yet how severe this bug is.
Minor fixes, would land the second part without this patch.
Attachment #522869 - Flags: review?(benjamin)
This is a patch khuey wrote for bug 493146, with some minor style nits changed.  I understand this problem somewhat better now and feel more comfortable reviewing this patch (it's actually a pretty trivial change).

And if it's wrong, it'll blow up the deadlock detector ;).
Attachment #522871 - Flags: review+
Blocks: 646398
Adding myself to CC since I was involved in reviewing bug #493146 and also introduced the nsCacheService::SyncWithCacheIOThread() mechanism which seems to be the culprit here.

A question to clarify: Does this show up only with the patch by khuey from bug #493146 (i.e. does it enable the deadlock-detection)? Or does that patch resolve the issue seen here?

Are there any examples of this actually causing Fx to deadlock in practice btw?
Attachment #522869 - Flags: review?(benjamin) → review+
(In reply to comment #4)
> Does this show up only with the patch by khuey from bug
> #493146 (i.e. does it enable the deadlock-detection)? Or does that patch
> resolve the issue seen here?

Well, the assertion failure here shows up after enabling lock-order checking of the mutex+monitor in nsCacheService, which is a patch that khuey wrote, and khuey also wrote a fix for the new assertion failure.  The latter is the patch attached here.

> Are there any examples of this actually causing Fx to deadlock in practice btw?

Not sure.
Increased frequency of intermittent orange bug 618052, backed out

http://hg.mozilla.org/mozilla-central/rev/e03c3a6df3cb

Sigh.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/1a89509e25e4
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
With M-C of 6 Apr 2011 I can no longer repro this either on Helgrind
(iow, independent confirmation of fix)
Assignee: nobody → jones.chris.g
You need to log in before you can comment on or make changes to this bug.