Bug 796160 – Race when client sends some udp packets before play request

After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.

Bug 796160 - Race when client sends some udp packets before play request


Summary:	Race when client sends some udp packets before play request


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-rtsp-server
Version:	git master
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	1.14.2
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2018-05-16 07:16 UTC by Göran Jönsson
Modified:	2018-06-27 10:54 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
patch in gst-rtsp-server ( minimum for solving problem ) (1.25 KB, patch) 2018-05-16 07:16 UTC, Göran Jönsson	none	Details \| Review
patch in gst-rtsp-server ( refactor ) (4.01 KB, patch) 2018-05-16 07:17 UTC, Göran Jönsson	none	Details \| Review
gst-rtsp-server patch (1.74 KB, patch) 2018-06-27 09:56 UTC, Göran Jönsson	committed	Details \| Review

Description Göran Jönsson 2018-05-16 07:16:15 UTC

Created attachment 372108 [details] [review]
patch in gst-rtsp-server ( minimum for solving problem )

There is a race in plug_src in rtsp-stream between setting src to 
playing and linking the pads, when src is udpsrc and client sending 
udp data before play. Then the udpsrc want to push data on src pad before it's 
linked this causing a link error.

We have come across a client doing that.

When client is not sending udp data the thread caused by setting 
udp src to playing is stuck at.

+ Trace 238617

#0 poll
at ../sysdeps/unix/syscall-template.S line 81
#1 g_socket_condition_timed_wait
from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#2 gst_udpsrc_fill
at gstudpsrc.c line 877
#3 gst_push_src_fill
at gstpushsrc.c line 174
#4 gst_base_src_default_create
at gstbasesrc.c line 1525
#5 gst_push_src_create
at gstpushsrc.c line 137
#6 gst_base_src_get_range
at gstbasesrc.c line 2528
#7 gst_base_src_loop
at gstbasesrc.c line 2875
#8 gst_task_func
at gsttask.c line 332
#9 ??
from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 ??
from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 start_thread
at pthread_create.c line 309
#12 clone
at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 111


This is not the case when client sending udp data just befor play request !!!

One solution is to remove the strange part in plug_src were setting a 
single element to playing before adding to bin. Instead we sync
the element with the state in bin after it have been added to bin.  

Please find attachment
(Fix problem) 
0001-rtsp-stream-Give-all-elements-in-pipeline-same-statu.patch
(Refactor)
0002-rtsp-stream-Refactor-plug_src.patch

Comment 1 Göran Jönsson 2018-05-16 07:17:36 UTC

Created attachment 372111 [details] [review]
patch in gst-rtsp-server ( refactor )

Comment 2 Göran Jönsson 2018-05-18 04:58:46 UTC

Any news about this ?

Comment 3 Göran Jönsson 2018-05-24 11:18:14 UTC

Any news about this ?

Comment 4 Göran Jönsson 2018-05-31 05:34:49 UTC

Any news about this ?

Comment 5 Göran Jönsson 2018-06-05 06:15:30 UTC

Any news about this ?

Comment 6 Göran Jönsson 2018-06-11 07:07:52 UTC

There might be a perfomance problem with this patch. i will get back when I know more.

Comment 7 Göran Jönsson 2018-06-21 12:15:00 UTC

There are no performance problem with this patch. The root cause of the performance problem is in our downstream code.

Comment 8 Tim-Philipp Müller 2018-06-26 11:32:42 UTC

Could one also solve this by simply blocking the udpsrc's src pad before setting it to PLAYING state and unblocking again after it has been linked to the funnel? Then we don't need to touch this state code and it seems to be a more direct way of addressing the issue.

Comment 9 Göran Jönsson 2018-06-27 09:56:04 UTC

Created attachment 372846 [details] [review]
gst-rtsp-server patch

Attached a new patch.

Comment 10 Tim-Philipp Müller 2018-06-27 10:53:48 UTC

Thanks, fixed indentation and rephrased wording in the commit message and comment a bit:

commit c1fab570d8e845685b7e890b2a588314382c1ded (HEAD -> master)
Author: Göran Jönsson <goranjn@axis.com>
Date:   Wed Jun 27 08:30:42 2018 +0200

    rtsp-stream: avoid pushing data on unlinked udpsrc pad during setup
    
    Fix race when setting up source elements.
    
    Since we set the source element(s) to PLAYING state before hooking
    them up to the downstream funnel, it's possible for the source element
    to receive packets before we actually get to linking it to the funnel,
    in which case buffers would be pushed out on an unlinked pad, causing
    it to error out and stop receiving more data.
    
    We fix this by blocking the source's srcpad until we have linked it.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796160