Bug 1677555 - Always release imgRequestProxy async. r=tnikkel

In this particular case the issue wouldn't end up in any sort of memory
corruption if we didn't safely crash, but these are quite tricky to
reason about, so it's better to avoid the reentrancy altogether if
possible.

I tried to convert the fuzzer test-case in a crashtest but failed (as
in, it didn't crash without the patch under the test harness).

Differential Revision: https://phabricator.services.mozilla.com/D115943
This commit is contained in:
Emilio Cobos Álvarez 2021-05-26 11:08:10 +00:00
parent 6f5b8aa349
commit a701adee41
2 changed files with 11 additions and 6 deletions

View File

@ -84,6 +84,10 @@ void ImageLoader::Init() {
/* static */
void ImageLoader::Shutdown() {
for (const auto& entry : *sImages) {
entry.GetKey()->CancelAndForgetObserver(NS_BINDING_ABORTED);
}
delete sImages;
sImages = nullptr;
sImageObserver = nullptr;
@ -449,6 +453,10 @@ void ImageLoader::UnloadImage(imgRequestProxy* aImage) {
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aImage);
if (MOZ_UNLIKELY(!sImages)) {
return; // Shutdown() takes care of it.
}
auto lookup = sImages->Lookup(aImage);
MOZ_DIAGNOSTIC_ASSERT(lookup, "Unregistered image?");
if (MOZ_UNLIKELY(!lookup)) {

View File

@ -188,13 +188,10 @@ class StyleImageRequestCleanupTask final : public mozilla::Runnable {
// This is defined here for parallelism with LoadURI.
void Gecko_LoadData_Drop(StyleLoadData* aData) {
if (aData->resolved_image) {
// We want to dispatch this async to prevent reentrancy issues, as
// imgRequestProxy going away can destroy documents, etc, see bug 1677555.
auto task = MakeRefPtr<StyleImageRequestCleanupTask>(*aData);
if (NS_IsMainThread()) {
task->Run();
} else {
// if Resolve was not called at some point, mDocGroup is not set.
SchedulerGroup::Dispatch(TaskCategory::Other, task.forget());
}
SchedulerGroup::Dispatch(TaskCategory::Other, task.forget());
}
// URIs are safe to refcount from any thread.