libparsec's client_start while client_stop is running on the same device not properly handled

[touilleMan]

parsec-cloud/libparsec/src/client.rs

Lines 208 to 231 in 8bfd773

	pub async fn client_stop(client: Handle) -> Result<(), ClientStopError> {
	let client_handle = client;
	let (client, on_event, device_in_use_guard) =
	take_and_close_handle(client_handle, |x| match x {
	HandleItem::Client {
	client,
	on_event,
	#[cfg(not(target_arch = "wasm32"))]
	device_in_use_guard,
	} => {
	#[cfg(target_arch = "wasm32")]
	let device_in_use_guard = ();
	Ok((client, on_event, device_in_use_guard))
	}
	// Note we consider an error if the handle is in `HandleItem::StartingClient` state
	// this is because at that point this is not a legit use of the handle given it
	// has never been yet provided to the caller in the first place !
	// On top of that it simplifies the start logic (given it guarantees nothing will
	// concurrently close the handle)
	invalid => Err(invalid),
	})?;
	// Note stopping the client also stop all it related workspace ops
	client.stop().await;

So client_stop first unregisters the handle, then proceeds to do the actual closing of the client.
On top of that the closing involve asynchronous operation (i.e. client.stop().await) which may take time.

So if a concurrent client_start comes for the same device, it will try to start the client right away and:

• on Native end up with an "device already in use" error given the IPC device_in_use_guard is still held while the client is being stopped.
• on web this will most likely succeed (ending up with two clients using the same device for a short time)

It would be better to instead have a dedicated error for client being stopped with an event as parameter to know when the stop is done (this is basically what we already do when calling a client_start for a device that is currently being started).

To do that I guess we would need to introduce a take_and_close_handle_with_teardown just like we have a register_handle_with_init