[Bug 764744] Crashes when multiple udpsrc are created for each client on a shared media, misses tracking and cleanup

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=764744

--- Comment #17 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 326051:
 --> (https://bugzilla.gnome.org/review?bug=764744&attachment=326051)

For a unit test, adding/removing clients and checking if things still work
afterwards would be a good test I guess... also checking if all but the first
client properly work

Check the existing tests for ideas :)

::: gst/rtsp-server/rtsp-stream.c
@@ +264,3 @@
   priv->ptmap = g_hash_table_new_full (NULL, NULL, NULL,
       (GDestroyNotify) gst_caps_unref);
+  priv->udpsrcs = g_hash_table_new (g_direct_hash, g_direct_equal);

You probably want to create the hash table with a free function, so that you
don't have to manually free the items when they get removed

@@ +1467,3 @@
+        g_slice_new0 (GstRTSPStreamUDPSrcs);
+    transport_udpsrcs->udpsrc[0] = NULL;
+    transport_udpsrcs->udpsrc[1] = NULL;

They are already NULL because you allocate them with new0()

@@ +1480,3 @@
+      /* TODO: Seems like this should be the same as the IPV4 unicast case, so 
+       * I've commented out this check that prevented multiple IPV6 unicast
udpsrcs. 
+       * This could be incorrect. */

It seems correct, they should behave the same

@@ +1494,3 @@
+     * Otherwise, add it to the hash table */
+    if (transport_udpsrcs->udpsrc[0] == NULL
+        && transport_udpsrcs->udpsrc[1] == NULL)

I think udpsrc[1] could be NULL if RTCP is disabled, or not?

@@ +2517,3 @@
+ * postponed.
+ * See https://bugzilla.gnome.org/show_bug.cgi?id=757488 */
+#ifdef NOTHING

I think you're right, also for the other one below. This code can disappear,
it's now done in play_udpsources_one_family()

@@ +3305,3 @@

+/* Properly dispose udpsrcs that were created for a given transport. */
+/* Must be called with a lock. */

Which lock? :) Better to be explicit here in the comments

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.