Skip to content

Commit

Permalink
(history): Drop states borrow before callback invocation
Browse files Browse the repository at this point in the history
To prevent situations of 'already mutably borrowed: BorrowError'
when invoking callbacks from BrowserHistory where state is in
use, drop the borrow before invoking the callbacks.

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
  • Loading branch information
kinnison committed Jan 18, 2023
1 parent d6d00c3 commit 08b0894
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
5 changes: 5 additions & 0 deletions crates/history/src/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl History for BrowserHistory {
.push_state_with_url(&history_state, "", Some(&url))
.expect_throw("failed to push state.");

drop(states);
self.notify_callbacks();
}

Expand All @@ -98,10 +99,12 @@ impl History for BrowserHistory {

let mut states = self.states.borrow_mut();
states.insert(id, Rc::new(state) as Rc<dyn Any>);

self.inner
.replace_state_with_url(&history_state, "", Some(&url))
.expect_throw("failed to replace state.");

drop(states);
self.notify_callbacks();
}

Expand Down Expand Up @@ -170,6 +173,7 @@ impl History for BrowserHistory {
.push_state_with_url(&history_state, "", Some(&url))
.expect_throw("failed to push history.");

drop(states);
self.notify_callbacks();
Ok(())
}
Expand Down Expand Up @@ -199,6 +203,7 @@ impl History for BrowserHistory {
.replace_state_with_url(&history_state, "", Some(&url))
.expect_throw("failed to replace history.");

drop(states);
self.notify_callbacks();
Ok(())
}
Expand Down
13 changes: 13 additions & 0 deletions crates/history/tests/browser_history.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::rc::Rc;

use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure};

use gloo_history::{BrowserHistory, History};
Expand Down Expand Up @@ -42,4 +44,15 @@ async fn history_works() {
let history = history.clone();
delayed_assert_eq(move || history.location().path().to_owned(), || "/path-b").await;
}

let _listener = history.listen({
let history = history.clone();
move || {
let location = history.location();
let state: Option<Rc<String>> = location.state();
assert_eq!(state, Some(Rc::new(location.path().to_owned())));
}
});

history.push_with_state("/fish", String::from("/fish"));
}

0 comments on commit 08b0894

Please sign in to comment.