902194 - Investigate whether or not EncodedBufferCache needs a reentrant lock.

Status: RESOLVED FIXED

(Core :: Audio/Video: Recording, defect)

Description

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

EncodedBufferCache uses a ReentrantMonitor.  This allows the same thread to lock the monitor multiple times without deadlocking.  If that is unnecessary we should replace it with a regular Mutex.  I couldn't see any obvious paths for reentrancy, but it's possible I missed something.

I originally noticed this because of the unnecessary forward declaration at http://hg.mozilla.org/mozilla-central/annotate/3ea0cb5a747e/content/media/EncodedBufferCache.h#l17.  We should remove that even if the ReentrantMonitor is needed.

Comment 1

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

umm, I think it can use a mutex to replace the ReentrantMonitor.
Protect the mEncodedBuffers/mDataSize.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Please do that :-)

Comment 3

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v1

Use MutexAutoLock to replace the ReentrantMonitor.

Comment 4

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

try result
https://tbpl.mozilla.org/?tree=Try&rev=a083bd250759

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/328f5dc01949

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/328f5dc01949