remove FutureId.assign_existing

This commit is contained in:
teoxoy 2024-07-02 18:05:49 +02:00 committed by Teodor Tanasoaia
parent cd9f003477
commit 39a268c624
3 changed files with 4 additions and 48 deletions

View File

@ -834,20 +834,6 @@ impl Global {
Err(e) => break 'error e, Err(e) => break 'error e,
}; };
// Currently we make a distinction between fid.assign and fid.assign_existing. This distinction is incorrect,
// but see https://github.com/gfx-rs/wgpu/issues/4912.
//
// `assign` also registers the ID with the resource info, so it can be automatically reclaimed. This needs to
// happen with a mutable reference, which means it can only happen on creation.
//
// Because we need to call `assign` inside the closure (to get mut access), we need to "move" the future id into the closure.
// Rust cannot figure out at compile time that we only ever consume the ID once, so we need to move the check
// to runtime using an Option.
let mut fid = Some(fid);
// The closure might get called, and it might give us an ID. Side channel it out of the closure.
let mut id = None;
let bgl_result = device.bgl_pool.get_or_init(entry_map, |entry_map| { let bgl_result = device.bgl_pool.get_or_init(entry_map, |entry_map| {
let bgl = let bgl =
device.create_bind_group_layout(&desc.label, entry_map, bgl::Origin::Pool)?; device.create_bind_group_layout(&desc.label, entry_map, bgl::Origin::Pool)?;
@ -856,8 +842,6 @@ impl Global {
.unwrap(); .unwrap();
let bgl = Arc::new(bgl); let bgl = Arc::new(bgl);
let id_inner = fid.take().unwrap().assign(bgl.clone());
id = Some(id_inner);
Ok(bgl) Ok(bgl)
}); });
@ -867,16 +851,10 @@ impl Global {
Err(e) => break 'error e, Err(e) => break 'error e,
}; };
// If the ID was not assigned, and we survived the above check, let id = fid.assign(layout.clone());
// it means that the bind group layout already existed and we need to call `assign_existing`.
//
// Calling this function _will_ leak the ID. See https://github.com/gfx-rs/wgpu/issues/4912.
if id.is_none() {
id = Some(fid.take().unwrap().assign_existing(&layout))
}
api_log!("Device::create_bind_group_layout -> {id:?}"); api_log!("Device::create_bind_group_layout -> {id:?}");
return (id.unwrap(), None); return (id, None);
}; };
let fid = hub.bind_group_layouts.prepare(id_in); let fid = hub.bind_group_layouts.prepare(id_in);
@ -1705,7 +1683,7 @@ impl Global {
Err(_) => break 'error binding_model::GetBindGroupLayoutError::InvalidPipeline, Err(_) => break 'error binding_model::GetBindGroupLayoutError::InvalidPipeline,
}; };
let id = match pipeline.layout.bind_group_layouts.get(index as usize) { let id = match pipeline.layout.bind_group_layouts.get(index as usize) {
Some(bg) => hub.bind_group_layouts.prepare(id_in).assign_existing(bg), Some(bg) => hub.bind_group_layouts.prepare(id_in).assign(bg.clone()),
None => { None => {
break 'error binding_model::GetBindGroupLayoutError::InvalidGroupIndex(index) break 'error binding_model::GetBindGroupLayoutError::InvalidGroupIndex(index)
} }
@ -1906,7 +1884,7 @@ impl Global {
}; };
let id = match pipeline.layout.bind_group_layouts.get(index as usize) { let id = match pipeline.layout.bind_group_layouts.get(index as usize) {
Some(bg) => hub.bind_group_layouts.prepare(id_in).assign_existing(bg), Some(bg) => hub.bind_group_layouts.prepare(id_in).assign(bg.clone()),
None => { None => {
break 'error binding_model::GetBindGroupLayoutError::InvalidGroupIndex(index) break 'error binding_model::GetBindGroupLayoutError::InvalidGroupIndex(index)
} }

View File

@ -82,16 +82,6 @@ impl<T: StorageItem> FutureId<'_, T> {
self.id self.id
} }
/// Assign an existing resource to a new ID.
///
/// Registers it with the registry.
pub fn assign_existing(self, value: &Arc<T>) -> Id<T::Marker> {
let mut data = self.data.write();
debug_assert!(!data.contains(self.id));
data.insert(self.id, value.clone());
self.id
}
pub fn assign_error(self) -> Id<T::Marker> { pub fn assign_error(self) -> Id<T::Marker> {
self.data.write().insert_error(self.id); self.data.write().insert_error(self.id);
self.id self.id

View File

@ -80,18 +80,6 @@ impl<T> Storage<T>
where where
T: StorageItem, T: StorageItem,
{ {
#[allow(dead_code)]
pub(crate) fn contains(&self, id: Id<T::Marker>) -> bool {
let (index, epoch, _) = id.unzip();
match self.map.get(index as usize) {
Some(&Element::Vacant) => false,
Some(&Element::Occupied(_, storage_epoch) | &Element::Error(storage_epoch)) => {
storage_epoch == epoch
}
None => false,
}
}
/// Get a reference to an item behind a potentially invalid ID. /// Get a reference to an item behind a potentially invalid ID.
/// Panics if there is an epoch mismatch, or the entry is empty. /// Panics if there is an epoch mismatch, or the entry is empty.
pub(crate) fn get(&self, id: Id<T::Marker>) -> Result<&Arc<T>, InvalidId> { pub(crate) fn get(&self, id: Id<T::Marker>) -> Result<&Arc<T>, InvalidId> {