IceDBorn / pipewire-screenaudio

Extension to passthrough pipewire audio to WebRTC Screenshare
https://addons.mozilla.org/firefox/addon/pipewire-screenaudio/
GNU General Public License v3.0
167 stars 5 forks source link

connect-and-monitor.sh crashes because it fails to link (race condition) #43

Closed alansartorio closed 1 year ago

alansartorio commented 1 year ago

While sharing, connect-and-monitor.sh process crashed. In the logs, I was able to see this:

[connect-and-monitor.sh] [Got New Node ID] ID: 271
[connect-and-monitor.sh] [Got Ports IDs] FL IDs: , FR IDs:

This is most probably a race condition, because we do: ask for node info -> ask for a full pw-dump to get it's ports -> connect them.

The node can (and probably did) get removed between any of those steps, and it will crash when it reaches pw-link, which returns error exit status when it can't connect. We either:

  1. Prevent this race condition from occurring between the first and second step by unifying them and getting pw-dump since the beginning and then query that for getting node info and port ids. (kinda costly doing full pw-dump for every node)
  2. Detect the race condition by checking if we successfully got the two ports in the second step (they shouldn't be empty strings).

But then if we lose the node before linking the ports, pw-link fails and because we've set set -e, the script will crash. So I think we should also ignore pw-link exit status code (pw-link ... || true) because we cant prevent this race condition.

jim3692 commented 1 year ago

My suggestion is to implement a temporary solution (like checking for empty strings, or ignoring the exit status of pw-link) and have a fully working solution on 1.0.0 where we will not rely on pw-dump

alansartorio commented 1 year ago

I agree