Move fence synchronisation into wgpu-hal

[Vecvec]

Connections
Fixes #9470

Description
Previously, queue submit wrote to the fence in wgpu-core. However, device.poll read from the same fence, meaning that queue.submit could not execute while device.poll was running. This could be a problem in a multi-threaded environment with long execution times of command buffer. This PR moves the locks into wgpu-hal (where needed) so that they can be dropped when waiting. Also fixes a deadlock exposed because the fence write

One thing that I don't like is the requirement for Arc<RwLock<Fence>>s inside a RwLock<Vec<...>>s for vulkan and openGL, but I could not find any way to remove this. The inner RwLocks are to stop the backend from resetting fences that are being waited on (the wait here should be minimal because the fences are already determined to be signalled) and the Arcs are to allow them to be kept while outside of the main lock (such as in wait).

The lock on the fence in core was not removed. It seems that present uses it to exclude other queue operations. @inner-daemons, is the only operation this needs to exclude queue.submit? I'm happy to complete that in this PR, or wait it until another PR.

Testing
Existing tests run the same as previously. Solves the issue.

Squash or Rebase?

Squash

Checklist

• I self-reviewed and fully understand this PR.
• WebGPU implementations built with wgpu may be affected behaviorally. (At least, in terms of speed)
• Validation and feature gates are in place to confine behavioral changes.
• Tests demonstrate the validation and altered logic works.
• CHANGELOG.md entries for the user-facing effects of this change are present.
• The PR is minimal, and doesn't make sense to land as multiple PRs. The deadlock is so common that I think the PR would be broken otherwise (I usually deadlocked within 10 frames [one submit per frame, the other thread only submitted 20 times per second])
• Commits are logically scoped and individually reviewable.
• The PR description has enough context to understand the motivation and solution implemented.

[Vecvec]

After rebasing, the code that was causing the deadlock appears to have been refactored, so ignore the comments about a deadlock.

Edit: was #9307

[andyleiserson]

I don't particularly like that things are set up this way (or at least, I think it should be documented more clearly what is going on), but I believe that the fence RwLock is being used not just to protect &mut self methods on the fence, but also to ensure mutual exclusion between submit and other things that don't want a submit to happen concurrently. Which will no longer be the case if submit only acquires a read lock.

I also don't particularly like that there are separate locks for the fence and command indices (I feel like the Fence could also have responsibility for giving out command indices), nor do I like that the protection against concurrent submits (see https://github.com/gfx-rs/wgpu/pull/9307/changes#diff-150156a37cf3627465ceb22096ed995ee26ae640007c421e726134bafd499dbeR1679-R1683) is more pessimistic than necessary (the validate_command_buffers processing probably could be done concurrently). But looking for solutions that don't bite off too much refactoring, one strategy might be to switch to using the command indices lock rather than the fence lock to provide mutual exclusion with concurrent submits (and document this, since it's non-obvious). If we do that, then I think we could get rid of the fence lock in wgpu_core entirely and rely on the locking in hal.

[Vecvec]

I believe that the fence RwLock is being used not just to protect &mut self methods on the fence, but also to ensure mutual exclusion between submit and other things that don't want a submit to happen concurrently. Which will no longer be the case if submit only acquires a read lock.

I had assumed (at least for submission) that command indices was held for that purpose. The only thing which appeared to use fences for exclusion was present, which should still exclude due to using a write.

[andyleiserson]

The ones I'm worried about are:

wgpu/wgpu-core/src/device/resource.rs

Lines 5223 to 5225 in fa37706

	// Wait for all work to finish before configuring the surface.
	let snatch_guard = self.snatchable_lock.read();
	let fence = self.fence.read();

wgpu/wgpu-core/src/present.rs

Lines 168 to 176 in fa37706

	let fence = device.fence.read();
	let suf = self.raw(device.backend()).unwrap();
	let (texture, status) = match unsafe {
	suf.acquire_texture(
	Some(core::time::Duration::from_millis(FRAME_TIMEOUT_MS as u64)),
	fence.as_ref(),
	)
	} {

[Vecvec]

Do these require no queue work to be submitted? to me it seems that these shouldn't require queue locking -aquire uses it to wait for it's previous usage to be finished and configure talks about guarding against queue submissions:

wgpu/wgpu-core/src/device/resource.rs

Lines 5235 to 5236 in fa37706

	// After the wait, the queue should be empty. It can only be non-empty
	// if another thread is submitting at the same time.

I think that this probably requires an issue for going through and documenting or switching to use a RwLock<()> for exclusion.

[Vecvec]

The other way for Vulkan & OpenGl that I can see would probably be having a Fence wrapper that destroyed (or reset on Vulkan) the fence when dropped. It couldn't be clone as I think it would have to be unsafe, but it would remove the need for the lock. However, for Vulkan at least, the free list would have to have an Arc<RwLock<...>> around it to allow the drop impl to add the item back.

Edit: If unsafe was fine the lock could be forgotten and then force unlocked. That wouldn't require the Arc at least.