[pulseaudio-discuss] [PATCH 20/25] raop: Cleanup pollfd when the RTSP state enters TEARDOWN

[Hajime Fujita]

It seems that STATE_DISCONNECTED never happens in usual cases,
which left pollfd uncleaned even after the socket fds were
closed in the current version. This led to PulseAudio hang up when
starting playback after the previous connection is torn down.

This patch fixes the issue, thus fixes the hang up issue in such
a case.

Also, control and timing fds are registered in rtpoll, so they
should be closed after rtpoll is disabled. This is what the TCP
implementation does, so this patch also follows the same way.
---
 src/modules/raop/module-raop-sink.c | 11 +++++++--
 src/modules/raop/raop_client.c      | 45 ++++++++++++++++++++-----------------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/modules/raop/module-raop-sink.c b/src/modules/raop/module-raop-sink.c
index cc8bff4..bd9963b 100644
--- a/src/modules/raop/module-raop-sink.c
+++ b/src/modules/raop/module-raop-sink.c
@@ -355,8 +355,6 @@ static int udp_sink_process_msg(pa_msgobject *o, int code, void *data, int64_t o
             pollfd->events = POLLIN | POLLPRI;
             pollfd->revents = 0;
 
-            u->udp_control_fd = -1;
-            u->udp_timing_fd = -1;
             return 0;
         }
 
@@ -384,8 +382,12 @@ static int udp_sink_process_msg(pa_msgobject *o, int code, void *data, int64_t o
                 pa_module_unload_request(u->module, true);
             }
 
+            pa_close(u->udp_control_fd);
+            pa_close(u->udp_timing_fd);
+
             u->udp_control_fd = -1;
             u->udp_timing_fd = -1;
+
             return 0;
         }
     }
@@ -471,6 +473,11 @@ static void udp_disconnected_cb(void *userdata) {
 
     pa_assert(u);
 
+    /* This callback function is called from both STATE_TEARDOWN and
+       STATE_DISCONNECTED in raop_client.c */
+
+    pa_assert(u);
+
     pa_log_debug("Connection closed, informing IO thread...");
 
     pa_asyncmsgq_post(u->thread_mq.inq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_UDP_DISCONNECTED, NULL, 0, NULL, NULL);
diff --git a/src/modules/raop/raop_client.c b/src/modules/raop/raop_client.c
index ba79759..527f4b5 100644
--- a/src/modules/raop/raop_client.c
+++ b/src/modules/raop/raop_client.c
@@ -922,6 +922,8 @@ static void udp_rtsp_cb(pa_rtsp_client *rtsp, pa_rtsp_state state, pa_headerlist
 
         case STATE_TEARDOWN: {
             pa_log_debug("RAOP: TEARDOWN");
+            pa_assert(c->udp_disconnected_callback);
+            pa_assert(c->rtsp);
 
             pa_rtsp_disconnect(c->rtsp);
 
@@ -929,26 +931,35 @@ static void udp_rtsp_cb(pa_rtsp_client *rtsp, pa_rtsp_state state, pa_headerlist
                 pa_close(c->udp_stream_fd);
                 c->udp_stream_fd = -1;
             }
-            if (c->udp_control_fd > 0) {
-                pa_close(c->udp_control_fd);
-                c->udp_control_fd = -1;
-            }
-            if (c->udp_timing_fd > 0) {
-                pa_close(c->udp_timing_fd);
-                c->udp_timing_fd = -1;
-            }
 
-            pa_log_debug("RTSP control channel closed");
+            pa_log_debug("RTSP control channel closed (teardown)");
 
             pa_rtsp_client_free(c->rtsp);
             pa_xfree(c->sid);
             c->rtsp = NULL;
             c->sid = NULL;
 
+            /*
+              Callback for cleanup -- e.g. pollfd
+
+              Share the disconnected callback since TEARDOWN event
+              is essentially equivalent to DISCONNECTED.
+              In case some special treatment turns out to be required
+              for TEARDOWN in future, a new callback function may be
+              defined and used.
+            */
+            c->udp_disconnected_callback(c->udp_disconnected_userdata);
+
+            /* Control and timing fds are closed by udp_sink_process_msg,
+               after it disables poll */
+            c->udp_control_fd = -1;
+            c->udp_timing_fd = -1;
+
             break;
         }
 
         case STATE_DISCONNECTED: {
+            pa_log_debug("RAOP: DISCONNECTED");
             pa_assert(c->udp_disconnected_callback);
             pa_assert(c->rtsp);
 
@@ -956,16 +967,8 @@ static void udp_rtsp_cb(pa_rtsp_client *rtsp, pa_rtsp_state state, pa_headerlist
                 pa_close(c->udp_stream_fd);
                 c->udp_stream_fd = -1;
             }
-            if (c->udp_control_fd > 0) {
-                pa_close(c->udp_control_fd);
-                c->udp_control_fd = -1;
-            }
-            if (c->udp_timing_fd > 0) {
-                pa_close(c->udp_timing_fd);
-                c->udp_timing_fd = -1;
-            }
 
-            pa_log_debug("RTSP control channel closed");
+            pa_log_debug("RTSP control channel closed (disconnected)");
 
             pa_rtsp_client_free(c->rtsp);
             pa_xfree(c->sid);
@@ -973,6 +976,10 @@ static void udp_rtsp_cb(pa_rtsp_client *rtsp, pa_rtsp_state state, pa_headerlist
             c->sid = NULL;
 
             c->udp_disconnected_callback(c->udp_disconnected_userdata);
+            /* Control and timing fds are closed by udp_sink_process_msg,
+               after it disables poll */
+            c->udp_control_fd = -1;
+            c->udp_timing_fd = -1;
 
             break;
         }
@@ -1092,8 +1099,6 @@ int pa_raop_client_teardown(pa_raop_client *c) {
 
     pa_assert(c);
 
-    /* This should be followed by a STATE_DISCONNECTED event
-     * which will take care of cleaning up everything */
     if (c->rtsp != NULL)
         rv = pa_rtsp_teardown(c->rtsp);
 
-- 
1.8.1.2