Potential server lockup in PCM drain

[borine]

Looking at #666, i've found a potential issue with the PCM Drain operation in the server. I don't think that this is actually happening in #666, so I've raised it as a new issue.

The problem is that if a transport encoder thread is cancelled while the drain SYNC is in progress, then the client event handler thread is never notified that the drain is complete and remains blocked in pthread_cond_wait() forever. This situation is almost impossible to produce artificially, so I can't give steps to reproduce it. A possible fix might be something like:

diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c
index e7eaa56..1483eec 100644
--- a/src/ba-transport-pcm.c
+++ b/src/ba-transport-pcm.c
@@ -130,6 +130,14 @@ void ba_transport_pcm_thread_cleanup(struct ba_transport_pcm *pcm) {
 	struct ba_transport *t = pcm->t;
 	struct ba_transport_thread *th = pcm->th;
 
+	/* The thread may have been cancelled while a PCM drain operation
+	 * is in progress. To prevent ba_transport_pcm_drain() from blocking
+	 * forever, we signal that drain is no longer in progress. */
+	pthread_mutex_lock(&pcm->mutex);
+	pcm->synced = true;
+	pthread_mutex_unlock(&pcm->mutex);
+	pthread_cond_signal(&pcm->cond);
+
 	/* For proper functioning of the transport, all threads have to be
 	 * operational. Therefore, if one of the threads is being cancelled,
 	 * we have to cancel all other threads. */

[borine]

Looking at the latest logs posted in #666 it is clear the bluealsa lockups captured in the earlier logs most likely were caused by this issue after all. A specific combination of events is required to trigger it: The remote (sink) device sends a AVDTP SUSPEND command which causes Bluez to change the transport state to IDLE, and so bluealsa stops the transport. If a client happens to be draining a PCM at the same time then ba_transport_pcm_drain() is blocked forever in pthread_cond_wait().

[arkq]

@borine many tanks for investigating this! I will apply the patch of course (but I'm bit busy with my personal matters lately...)