Skip to content

Commit 325f3a7

Browse files
vgelostwilkens
authored andcommitted
Fix AssetServer::get_asset_loader deadlock (bevyengine#2395)
# Objective Fixes a possible deadlock between `AssetServer::get_asset_loader` / `AssetServer::add_loader` A thread could take the `extension_to_loader_index` read lock, and then have the `server.loader` write lock taken in add_loader before it can. Then add_loader can't take the extension_to_loader_index lock, and the program deadlocks. To be more precise: ## Step 1: Thread 1 grabs the `extension_to_loader_index` lock on lines 138..139: https://github.com/bevyengine/bevy/blob/3a1867a92edc571b8f842bb1a96112dcbdceae4b/crates/bevy_asset/src/asset_server.rs#L133-L145 ## Step 2: Thread 2 grabs the `server.loader` write lock on line 107: https://github.com/bevyengine/bevy/blob/3a1867a92edc571b8f842bb1a96112dcbdceae4b/crates/bevy_asset/src/asset_server.rs#L103-L116 ## Step 3: Deadlock, since Thread 1 wants to grab `server.loader` on line 141...: https://github.com/bevyengine/bevy/blob/3a1867a92edc571b8f842bb1a96112dcbdceae4b/crates/bevy_asset/src/asset_server.rs#L133-L145 ... and Thread 2 wants to grab 'extension_to_loader_index` on lines 111..112: https://github.com/bevyengine/bevy/blob/3a1867a92edc571b8f842bb1a96112dcbdceae4b/crates/bevy_asset/src/asset_server.rs#L103-L116 ## Solution Fixed by descoping the extension_to_loader_index lock, since `get_asset_loader` doesn't need to hold the read lock on the extensions map for the duration, just to get a copyable usize. The block might not be needed, I think I could have gotten away with just inserting a `copied()` call into the chain, but I wanted to make the reasoning clear for future maintainers.
1 parent 8755ad4 commit 325f3a7

File tree

1 file changed

+7
-5
lines changed

1 file changed

+7
-5
lines changed

crates/bevy_asset/src/asset_server.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,13 @@ impl AssetServer {
134134
&self,
135135
extension: &str,
136136
) -> Result<Arc<Box<dyn AssetLoader>>, AssetServerError> {
137-
self.server
138-
.extension_to_loader_index
139-
.read()
140-
.get(extension)
141-
.map(|index| self.server.loaders.read()[*index].clone())
137+
let index = {
138+
// scope map to drop lock as soon as possible
139+
let map = self.server.extension_to_loader_index.read();
140+
map.get(extension).copied()
141+
};
142+
index
143+
.map(|index| self.server.loaders.read()[index].clone())
142144
.ok_or_else(|| AssetServerError::MissingAssetLoader {
143145
extensions: vec![extension.to_string()],
144146
})

0 commit comments

Comments
 (0)