Auto merge of #29005 - BurtonQin:fix-double-lock, r=jdm

components/net: Fix a double-lock in image_cache

<!-- Please describe your changes on the following line: -->

Add fn `add_listener_with_store` which requires `self.store.lock()` before calling.
Use this function instead of `add_listener` on L555 because `self.store.lock()` is called before.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [] `./mach build -d` does not report any errors
- [] `./mach test-tidy` does not report any errors
- [X] These changes fix #29003 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because the fix is straightforward.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2022-10-25 19:41:09 -04:00 committed by GitHub
commit d13aa55c2d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -546,13 +546,13 @@ impl ImageCache for ImageCacheImpl {
match cache_result { match cache_result {
ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(_)) => { ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(_)) => {
let store = self.store.lock().unwrap(); let mut store = self.store.lock().unwrap();
let id = store let id = *store
.pending_loads .pending_loads
.url_to_load_key .url_to_load_key
.get(&(url, origin, cors_setting)) .get(&(url, origin, cors_setting))
.unwrap(); .unwrap();
self.add_listener(*id, ImageResponder::new(sender, *id)); self.add_listener_with_store(&mut store, id, ImageResponder::new(sender, id));
}, },
ImageCacheResult::Pending(id) | ImageCacheResult::ReadyForRequest(id) => { ImageCacheResult::Pending(id) | ImageCacheResult::ReadyForRequest(id) => {
self.add_listener(id, ImageResponder::new(sender, id)); self.add_listener(id, ImageResponder::new(sender, id));
@ -567,18 +567,7 @@ impl ImageCache for ImageCacheImpl {
/// the responder will still receive the expected response. /// the responder will still receive the expected response.
fn add_listener(&self, id: PendingImageId, listener: ImageResponder) { fn add_listener(&self, id: PendingImageId, listener: ImageResponder) {
let mut store = self.store.lock().unwrap(); let mut store = self.store.lock().unwrap();
if let Some(load) = store.pending_loads.get_by_key_mut(&id) { self.add_listener_with_store(&mut store, id, listener);
if let Some(ref metadata) = load.metadata {
listener.respond(ImageResponse::MetadataLoaded(metadata.clone()));
}
load.add_listener(listener);
return;
}
if let Some(load) = store.completed_loads.values().find(|l| l.id == id) {
listener.respond(load.image_response.clone());
return;
}
warn!("Couldn't find cached entry for listener {:?}", id);
} }
/// Inform the image cache about a response for a pending request. /// Inform the image cache about a response for a pending request.
@ -661,3 +650,26 @@ impl ImageCache for ImageCacheImpl {
} }
} }
} }
impl ImageCacheImpl {
/// Require self.store.lock() before calling.
fn add_listener_with_store(
&self,
store: &mut ImageCacheStore,
id: PendingImageId,
listener: ImageResponder,
) {
if let Some(load) = store.pending_loads.get_by_key_mut(&id) {
if let Some(ref metadata) = load.metadata {
listener.respond(ImageResponse::MetadataLoaded(metadata.clone()));
}
load.add_listener(listener);
return;
}
if let Some(load) = store.completed_loads.values().find(|l| l.id == id) {
listener.respond(load.image_response.clone());
return;
}
warn!("Couldn't find cached entry for listener {:?}", id);
}
}