Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix re_renderer file watcher watching the same file several times #1463

Merged
merged 1 commit into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions crates/re_renderer/src/file_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub use self::file_server_impl::FileServer;

#[cfg(all(not(target_arch = "wasm32"), debug_assertions))] // non-wasm + debug build
mod file_server_impl {
use ahash::HashSet;
use ahash::{HashMap, HashSet};
use anyhow::Context as _;
use crossbeam::channel::Receiver;
use notify::{Event, RecommendedWatcher, RecursiveMode, Watcher};
Expand All @@ -112,6 +112,7 @@ mod file_server_impl {
pub struct FileServer {
watcher: RecommendedWatcher,
events_rx: Receiver<Event>,
file_watch_count: HashMap<PathBuf, usize>,
}

// Private details
Expand All @@ -130,7 +131,11 @@ mod file_server_impl {
}
})?;

Ok(Self { watcher, events_rx })
Ok(Self {
watcher,
events_rx,
file_watch_count: HashMap::default(),
})
}
}

Expand Down Expand Up @@ -183,6 +188,14 @@ mod file_server_impl {
) -> anyhow::Result<PathBuf> {
let path = std::fs::canonicalize(path.as_ref())?;

match self.file_watch_count.entry(path.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
*entry.get_mut() += 1;
return Ok(path);
}
std::collections::hash_map::Entry::Vacant(entry) => entry.insert(1),
};

self.watcher
.watch(
path.as_ref(),
Expand Down Expand Up @@ -214,6 +227,19 @@ mod file_server_impl {
/// Stops watching for file events at the given `path`.
pub fn unwatch(&mut self, path: impl AsRef<Path>) -> anyhow::Result<()> {
let path = std::fs::canonicalize(path.as_ref())?;

match self.file_watch_count.entry(path.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
*entry.get_mut() -= 1;
if *entry.get() == 0 {
entry.remove();
}
}
std::collections::hash_map::Entry::Vacant(_) => {
anyhow::bail!("The path {:?} was not or no longer watched", path);
}
};

self.watcher
.unwatch(path.as_ref())
.with_context(|| format!("couldn't unwatch file at {path:?}"))
Expand Down
2 changes: 1 addition & 1 deletion crates/re_sdk/src/file_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl FileWriter {

re_log::debug!("Saving file to {path:?}…");

let file = std::fs::File::create(&path).with_context(|| format!("Path: {:?}", path))?;
let file = std::fs::File::create(&path).with_context(|| format!("Path: {path:?}"))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh boy... here we go 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason clippy started complaining here suddenly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well yes it's the famous "oh god I don't want to fix this one otherwise I'll have to justify why it passes the CI" lol #1447

let mut encoder = re_log_types::encoding::Encoder::new(file)?;

let join_handle = std::thread::Builder::new()
Expand Down