Skip to content

Commit

Permalink
layout/context: Wrap image_cache_thread in a Mutex<> as well
Browse files Browse the repository at this point in the history
`SharedLayoutContext.image_cache_thread` didn't get the `Mutex<>`
treatment along with all the other channels in there, because
`ipc-channel::Sender` is presently inherently `Sync`. I believe this to
be an implementation accident though, that should be rectified in the
future -- not something users should actually rely on...

Note that sharing senders (be it `mpsc` or `ipc-channel`) in is probably
not a good idea anyway: just cloning them -- and letting them handle the
sharing internally -- should be both simpler and cheaper. But right now
that's how things are handled here; so let's go with the flow...
  • Loading branch information
antrik committed Nov 3, 2016
1 parent 291f393 commit 4d14f1e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
13 changes: 8 additions & 5 deletions components/layout/context.rs
Expand Up @@ -76,7 +76,7 @@ pub struct SharedLayoutContext {
pub style_context: SharedStyleContext,

/// The shared image cache thread.
pub image_cache_thread: ImageCacheThread,
pub image_cache_thread: Mutex<ImageCacheThread>,

/// A channel for the image cache to send responses to.
pub image_cache_sender: Mutex<ImageCacheChan>,
Expand Down Expand Up @@ -132,7 +132,8 @@ impl SharedLayoutContext {
debug_assert!(opts::get().output_file.is_some() || opts::get().exit_after_load);

// See if the image is already available
let result = self.image_cache_thread.find_image(url.clone(), use_placeholder);
let result = self.image_cache_thread.lock().unwrap()
.find_image(url.clone(), use_placeholder);

match result {
Ok(image) => return Some(image),
Expand All @@ -146,7 +147,7 @@ impl SharedLayoutContext {
// If we are emitting an output file, then we need to block on
// image load or we risk emitting an output file missing the image.
let (sync_tx, sync_rx) = ipc::channel().unwrap();
self.image_cache_thread.request_image(url, ImageCacheChan(sync_tx), None);
self.image_cache_thread.lock().unwrap().request_image(url, ImageCacheChan(sync_tx), None);
loop {
match sync_rx.recv() {
Err(_) => return None,
Expand All @@ -170,7 +171,8 @@ impl SharedLayoutContext {
.map(|img| ImageOrMetadataAvailable::ImageAvailable(img));
}
// See if the image is already available
let result = self.image_cache_thread.find_image_or_metadata(url.clone(),
let result = self.image_cache_thread.lock().unwrap()
.find_image_or_metadata(url.clone(),
use_placeholder);
match result {
Ok(image_or_metadata) => Some(image_or_metadata),
Expand All @@ -179,7 +181,8 @@ impl SharedLayoutContext {
// Not yet requested, async mode - request image or metadata from the cache
Err(ImageState::NotRequested) => {
let sender = self.image_cache_sender.lock().unwrap().clone();
self.image_cache_thread.request_image_and_metadata(url, sender, None);
self.image_cache_thread.lock().unwrap()
.request_image_and_metadata(url, sender, None);
None
}
// Image has been requested, is still pending. Return no image for this paint loop.
Expand Down
2 changes: 1 addition & 1 deletion components/layout_thread/lib.rs
Expand Up @@ -513,7 +513,7 @@ impl LayoutThread {
local_context_creation_data: Mutex::new(local_style_context_creation_data),
timer: self.timer.clone(),
},
image_cache_thread: self.image_cache_thread.clone(),
image_cache_thread: Mutex::new(self.image_cache_thread.clone()),
image_cache_sender: Mutex::new(self.image_cache_sender.clone()),
font_cache_thread: Mutex::new(self.font_cache_thread.clone()),
webrender_image_cache: self.webrender_image_cache.clone(),
Expand Down

0 comments on commit 4d14f1e

Please sign in to comment.