Bug 1862777 - Lock the list in UpdateShmBlocks. a=dmeehan (2fa10896) · Commits · Dan Ballard / Mullvad Browser

gfx/thebes/SharedFontList-impl.h

+3 −1

Original line number	Diff line number	Diff line
		@@ -347,8 +347,10 @@ class FontList {
		/**
		* Used by child processes to ensure all the blocks are registered.
		* Returns false on failure.
		* Pass aMustLock=true to take the gfxPlatformFontList lock during the
		* update (not required when calling from the constructor).
		*/
		[[nodiscard]] bool UpdateShmBlocks();
		[[nodiscard]] bool UpdateShmBlocks(bool aMustLock);

		/**
		* This makes a sync IPC call to get a shared block from the parent.

gfx/thebes/SharedFontList.cpp

+29 −14

Original line number	Diff line number	Diff line
		@@ -41,8 +41,12 @@ static double WSSDistance(const Face* aFace, const gfxFontStyle& aStyle) {

		void* Pointer::ToPtr(FontList* aFontList,
		size_t aSize) const MOZ_NO_THREAD_SAFETY_ANALYSIS {
		// On failure, we'll return null; callers need to handle this appropriately
		// (e.g. via fallback).
		void* result = nullptr;

		if (IsNull()) {
		return nullptr;
		return result;
		}

		// Ensure the list doesn't get replaced out from under us. Font-list rebuild
		@@ -53,9 +57,6 @@ void* Pointer::ToPtr(FontList* aFontList,
		gfxPlatformFontList::PlatformFontList()->Lock();
		}

		// On failure, we'll return null; callers need to handle this appropriately
		// (e.g. via fallback).
		void* result = nullptr;
		uint32_t blockIndex = Block();

		// If the Pointer refers to a block we have not yet mapped in this process,
		@@ -63,19 +64,23 @@ void* Pointer::ToPtr(FontList* aFontList,
		// our mBlocks list.
		auto& blocks = aFontList->mBlocks;
		if (blockIndex >= blocks.Length()) {
		if (XRE_IsParentProcess()) {
		if (MOZ_UNLIKELY(XRE_IsParentProcess())) {
		// Shouldn't happen! A content process tried to pass a bad Pointer?
		goto cleanup;
		}
		// If we're not on the main thread, we can't do the IPC involved in
		// UpdateShmBlocks; just let the lookup fail for now.
		if (!isMainThread) {
		goto cleanup;
		}
		// UpdateShmBlocks can fail, if the parent has replaced the font list with
		// a new generation. In that case we just return null, and whatever font
		// the content process was trying to use will appear unusable for now. It's
		// about to receive a notification of the new font list anyhow, at which
		// point it will flush its caches and reflow everything, so the temporary
		// failure of this font will be forgotten.
		// We also return null if we're not on the main thread, as we cannot safely
		// do the IPC messaging needed here.
		if (!isMainThread \|\| !aFontList->UpdateShmBlocks()) {
		// UpdateShmBlocks will take the platform font-list lock during the update.
		if (MOZ_UNLIKELY(!aFontList->UpdateShmBlocks(true))) {
		goto cleanup;
		}
		MOZ_ASSERT(blockIndex < blocks.Length(), "failure in UpdateShmBlocks?");
		@@ -85,7 +90,7 @@ void* Pointer::ToPtr(FontList* aFontList,
		// In at least some cases, however, this can occur transiently while the
		// font list is being rebuilt by the parent; content will then be notified
		// that the list has changed, and should refresh everything successfully.
		if (blockIndex >= blocks.Length()) {
		if (MOZ_UNLIKELY(blockIndex >= blocks.Length())) {
		goto cleanup;
		}
		}
		@@ -93,7 +98,7 @@ void* Pointer::ToPtr(FontList* aFontList,
		{
		// Don't create a pointer that's outside what the block has allocated!
		const auto& block = blocks[blockIndex];
		if (Offset() + aSize <= block->Allocated()) {
		if (MOZ_LIKELY(Offset() + aSize <= block->Allocated())) {
		result = static_cast<char*>(block->Memory()) + Offset();
		}
		}
		@@ -708,7 +713,7 @@ FontList::FontList(uint32_t aGeneration) {
		blocks.Clear();
		// Update in case of any changes since the initial message was sent.
		for (unsigned retryCount = 0; retryCount < 3; ++retryCount) {
		if (UpdateShmBlocks()) {
		if (UpdateShmBlocks(false)) {
		return;
		}
		// The only reason for UpdateShmBlocks to fail is if the parent recreated
		@@ -864,16 +869,26 @@ FontList::ShmBlock* FontList::GetBlockFromParent(uint32_t aIndex) {
		return new ShmBlock(std::move(newShm));
		}

		bool FontList::UpdateShmBlocks() {
		// We don't take the lock when called from the constructor, so disable thread-
		// safety analysis here.
		bool FontList::UpdateShmBlocks(bool aMustLock) MOZ_NO_THREAD_SAFETY_ANALYSIS {
		MOZ_ASSERT(!XRE_IsParentProcess());
		if (aMustLock) {
		gfxPlatformFontList::PlatformFontList()->Lock();
		}
		bool result = true;
		while (!mBlocks.Length() \|\| mBlocks.Length() < GetHeader().mBlockCount) {
		ShmBlock* newBlock = GetBlockFromParent(mBlocks.Length());
		if (!newBlock) {
		return false;
		result = false;
		break;
		}
		mBlocks.AppendElement(newBlock);
		}
		return true;
		if (aMustLock) {
		gfxPlatformFontList::PlatformFontList()->Unlock();
		}
		return result;
		}

		void FontList::ShareBlocksToProcess(nsTArray<base::SharedMemoryHandle>* aBlocks,