import changes from `dev' branch of rmottola/Arctic-Fox:

- Bug 1257063 - Don't destruct the runnable inside the lock when TaskQueue::Dispatch fails. r=bobbyholley. (1f6b254bb0)
- Bug 1202148 - Move current in only one location in Intervals.h, r=jya (2c98d86b10)
- Bug 1258673. Part 1 - cache mStreamOffset so we won't read at the wrong position when Other Read() interrupt the current Read(). r=jya. (87ab65cc30)
- Bug 1258673. Part 2 - since mStreamOffset is not updated until the end of MediaCacheStream::Read(), we have to pass stream offset to MediaCache::NoteBlockUsage explicitly to avoid hitting the assertion. r=jya. (f02806ea1c)
This commit is contained in:
2024-06-25 16:03:58 +08:00
parent fef8b08889
commit 9c8d43b8a2
5 changed files with 54 additions and 40 deletions
+13 -7
View File
@@ -20,13 +20,19 @@ FlushableTaskQueue::Flush()
nsresult
FlushableTaskQueue::FlushAndDispatch(already_AddRefed<nsIRunnable> aRunnable)
{
MonitorAutoLock mon(mQueueMonitor);
AutoSetFlushing autoFlush(this);
FlushLocked();
nsCOMPtr<nsIRunnable> r = dont_AddRef(aRunnable.take());
nsresult rv = DispatchLocked(r.forget(), IgnoreFlushing, AssertDispatchSuccess);
NS_ENSURE_SUCCESS(rv, rv);
AwaitIdleLocked();
nsCOMPtr<nsIRunnable> r = aRunnable;
{
MonitorAutoLock mon(mQueueMonitor);
AutoSetFlushing autoFlush(this);
FlushLocked();
nsresult rv = DispatchLocked(/* passed by ref */r, IgnoreFlushing, AssertDispatchSuccess);
NS_ENSURE_SUCCESS(rv, rv);
AwaitIdleLocked();
}
// If the ownership of |r| is not transferred in DispatchLocked() due to
// dispatch failure, it will be deleted here outside the lock. We do so
// since the destructor of the runnable might access TaskQueue and result
// in deadlocks.
return NS_OK;
}
+1 -6
View File
@@ -361,23 +361,18 @@ public:
ContainerType normalized;
ElemType current(aInterval);
bool inserted = false;
IndexType i = 0;
for (; i < mIntervals.Length(); i++) {
ElemType& interval = mIntervals[i];
if (current.Touches(interval)) {
current = current.Span(interval);
} else if (current.LeftOf(interval)) {
normalized.AppendElement(Move(current));
inserted = true;
break;
} else {
normalized.AppendElement(Move(interval));
}
}
if (!inserted) {
normalized.AppendElement(Move(current));
}
normalized.AppendElement(Move(current));
for (; i < mIntervals.Length(); i++) {
normalized.AppendElement(Move(mIntervals[i]));
}
+20 -16
View File
@@ -181,6 +181,7 @@ public:
// and thus hasn't yet been committed to the cache. The caller will
// call QueueUpdate().
void NoteBlockUsage(MediaCacheStream* aStream, int32_t aBlockIndex,
int64_t aStreamOffset,
MediaCacheStream::ReadMode aMode, TimeStamp aNow);
// Mark aStream as having the block, adding it as an owner.
void AddBlockOwnerAsReadahead(int32_t aBlockIndex, MediaCacheStream* aStream,
@@ -1622,8 +1623,8 @@ MediaCache::Truncate()
void
MediaCache::NoteBlockUsage(MediaCacheStream* aStream, int32_t aBlockIndex,
MediaCacheStream::ReadMode aMode,
TimeStamp aNow)
int64_t aStreamOffset,
MediaCacheStream::ReadMode aMode, TimeStamp aNow)
{
mReentrantMonitor.AssertCurrentThreadIn();
@@ -1640,7 +1641,7 @@ MediaCache::NoteBlockUsage(MediaCacheStream* aStream, int32_t aBlockIndex,
// The following check has to be <= because the stream offset has
// not yet been updated for the data read from this block
NS_ASSERTION(bo->mStreamBlock*BLOCK_SIZE <= bo->mStream->mStreamOffset,
NS_ASSERTION(bo->mStreamBlock*BLOCK_SIZE <= aStreamOffset,
"Using a block that's behind the read position?");
GetListForBlock(bo)->RemoveBlock(aBlockIndex);
@@ -1673,8 +1674,8 @@ MediaCache::NoteSeek(MediaCacheStream* aStream, int64_t aOldOffset)
if (cacheBlockIndex >= 0) {
// Marking the block used may not be exactly what we want but
// it's simple
NoteBlockUsage(aStream, cacheBlockIndex, MediaCacheStream::MODE_PLAYBACK,
now);
NoteBlockUsage(aStream, cacheBlockIndex, aStream->mStreamOffset,
MediaCacheStream::MODE_PLAYBACK, now);
}
++blockIndex;
}
@@ -2208,17 +2209,20 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
if (mClosed)
return NS_ERROR_FAILURE;
// Cache the offset in case it is changed again when we are waiting for the
// monitor to be notified to avoid reading at the wrong position.
auto streamOffset = mStreamOffset;
uint32_t count = 0;
// Read one block (or part of a block) at a time
while (count < aCount) {
uint32_t streamBlock = uint32_t(mStreamOffset/BLOCK_SIZE);
uint32_t offsetInStreamBlock =
uint32_t(mStreamOffset - streamBlock*BLOCK_SIZE);
uint32_t streamBlock = uint32_t(streamOffset/BLOCK_SIZE);
uint32_t offsetInStreamBlock = uint32_t(streamOffset - streamBlock*BLOCK_SIZE);
int64_t size = std::min<int64_t>(aCount - count, BLOCK_SIZE - offsetInStreamBlock);
if (mStreamLength >= 0) {
// Don't try to read beyond the end of the stream
int64_t bytesRemaining = mStreamLength - mStreamOffset;
int64_t bytesRemaining = mStreamLength - streamOffset;
if (bytesRemaining <= 0) {
// Get out of here and return NS_OK
break;
@@ -2246,7 +2250,7 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
MediaCache::ResourceStreamIterator iter(mResourceID);
while (MediaCacheStream* stream = iter.Next()) {
if (uint32_t(stream->mChannelOffset/BLOCK_SIZE) == streamBlock &&
mStreamOffset < stream->mChannelOffset) {
streamOffset < stream->mChannelOffset) {
streamWithPartialBlock = stream;
break;
}
@@ -2255,7 +2259,7 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
// We can just use the data in mPartialBlockBuffer. In fact we should
// use it rather than waiting for the block to fill and land in
// the cache.
int64_t bytes = std::min<int64_t>(size, streamWithPartialBlock->mChannelOffset - mStreamOffset);
int64_t bytes = std::min<int64_t>(size, streamWithPartialBlock->mChannelOffset - streamOffset);
// Clamp bytes until 64-bit file size issues are fixed.
bytes = std::min(bytes, int64_t(INT32_MAX));
MOZ_ASSERT(bytes >= 0 && bytes <= aCount, "Bytes out of range.");
@@ -2264,7 +2268,7 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
if (mCurrentMode == MODE_METADATA) {
streamWithPartialBlock->mMetadataInPartialBlockBuffer = true;
}
mStreamOffset += bytes;
streamOffset += bytes;
count = bytes;
break;
}
@@ -2279,7 +2283,7 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
continue;
}
gMediaCache->NoteBlockUsage(this, cacheBlock, mCurrentMode, TimeStamp::Now());
gMediaCache->NoteBlockUsage(this, cacheBlock, streamOffset, mCurrentMode, TimeStamp::Now());
int64_t offset = cacheBlock*BLOCK_SIZE + offsetInStreamBlock;
int32_t bytes;
@@ -2291,7 +2295,7 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
// If we did successfully read some data, may as well return it
break;
}
mStreamOffset += bytes;
streamOffset += bytes;
count += bytes;
}
@@ -2300,9 +2304,9 @@ MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
// have changed
gMediaCache->QueueUpdate();
}
CACHE_LOG(LogLevel::Debug,
("Stream %p Read at %lld count=%d", this, (long long)(mStreamOffset-count), count));
CACHE_LOG(LogLevel::Debug, ("Stream %p Read at %lld count=%d", this, streamOffset-count, count));
*aBytes = count;
mStreamOffset = streamOffset;
return NS_OK;
}
+7 -6
View File
@@ -39,15 +39,16 @@ TaskQueue::TailDispatcher()
return *mTailDispatcher;
}
// Note aRunnable is passed by ref to support conditional ownership transfer.
// See Dispatch() in TaskQueue.h for more details.
nsresult
TaskQueue::DispatchLocked(already_AddRefed<nsIRunnable> aRunnable,
DispatchMode aMode, DispatchFailureHandling aFailureHandling,
DispatchReason aReason)
TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
DispatchMode aMode, DispatchFailureHandling aFailureHandling,
DispatchReason aReason)
{
nsCOMPtr<nsIRunnable> r = aRunnable;
AbstractThread* currentThread;
if (aReason != TailDispatch && (currentThread = GetCurrent()) && RequiresTailDispatch(currentThread)) {
currentThread->TailDispatcher().AddTask(this, r.forget(), aFailureHandling);
currentThread->TailDispatcher().AddTask(this, aRunnable.forget(), aFailureHandling);
return NS_OK;
}
@@ -58,7 +59,7 @@ TaskQueue::DispatchLocked(already_AddRefed<nsIRunnable> aRunnable,
if (mIsShutdown) {
return NS_ERROR_FAILURE;
}
mTasks.push(r.forget());
mTasks.push(aRunnable.forget());
if (mIsRunning) {
return NS_OK;
}
+13 -5
View File
@@ -43,10 +43,17 @@ public:
DispatchFailureHandling aFailureHandling = AssertDispatchSuccess,
DispatchReason aReason = NormalDispatch) override
{
MonitorAutoLock mon(mQueueMonitor);
nsresult rv = DispatchLocked(Move(aRunnable), AbortIfFlushing, aFailureHandling, aReason);
MOZ_DIAGNOSTIC_ASSERT(aFailureHandling == DontAssertDispatchSuccess || NS_SUCCEEDED(rv));
Unused << rv;
nsCOMPtr<nsIRunnable> r = aRunnable;
{
MonitorAutoLock mon(mQueueMonitor);
nsresult rv = DispatchLocked(/* passed by ref */r, AbortIfFlushing, aFailureHandling, aReason);
MOZ_DIAGNOSTIC_ASSERT(aFailureHandling == DontAssertDispatchSuccess || NS_SUCCEEDED(rv));
Unused << rv;
}
// If the ownership of |r| is not transferred in DispatchLocked() due to
// dispatch failure, it will be deleted here outside the lock. We do so
// since the destructor of the runnable might access TaskQueue and result
// in deadlocks.
}
// Puts the queue in a shutdown state and returns immediately. The queue will
@@ -81,7 +88,8 @@ protected:
enum DispatchMode { AbortIfFlushing, IgnoreFlushing };
nsresult DispatchLocked(already_AddRefed<nsIRunnable> aRunnable, DispatchMode aMode,
nsresult DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
DispatchMode aMode,
DispatchFailureHandling aFailureHandling,
DispatchReason aReason = NormalDispatch);