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 765673 - udpsrc error: getsockname failed: could not get local address: Socket operation on non-socket
udpsrc error: getsockname failed: could not get local address: Socket operati...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.10.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-27 13:04 UTC by Patricia Muscalu
Modified: 2016-11-23 11:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test proved socket (1.96 KB, patch)
2016-04-27 13:04 UTC, Patricia Muscalu
rejected Details | Review
gst-rtsp-server patch (1.03 KB, patch)
2016-11-21 12:33 UTC, Göran Jönsson
none Details | Review
gst-rtsp-server patch (1.15 KB, patch)
2016-11-22 08:40 UTC, Göran Jönsson
committed Details | Review

Description Patricia Muscalu 2016-04-27 13:04:58 UTC
Created attachment 326867 [details] [review]
test proved socket

There is a problem when setting a "socket" property on an UDP source element and changing the state of the source between NULL and READY.

When going from NULL->READY:
src->used_socket = G_SOCKET (g_object_ref (src->socket))
is set udpsrc_open()

When going from READY->NULL:
src->used_socket is closed in udpsrc_close().
Note that the default close_socket option is set to true. This is exactly the same case in gst-rtsp-server where the UDP socketes are shared between UDP sources and UDP multisink. It's the UDP source owning the socket.

After changing the state back to READY again, the source will
try to use the provided socket that has been released in the previous state transition. Thus the 

I'm attaching a simple unit test that exposes this problem. The test simulates the same problem that can occur when running stress/stability tests where gst-rtsp-server is involved.
Comment 1 Sebastian Dröge (slomo) 2016-04-27 13:13:08 UTC
This basically means that whatever socket was provided from the application becomes invalid and unusable after going to NULL if close-socket=TRUE?
Comment 2 Patricia Muscalu 2016-04-27 13:17:11 UTC
Yes, it's true.
Comment 3 Patricia Muscalu 2016-04-27 13:22:36 UTC
Another problem is the udpsrc code is not thread safe. Settings the property should be protected and only allowed while being in the state <= READY.
Comment 4 Sebastian Dröge (slomo) 2016-04-27 13:24:04 UTC
Yes, you shouldn't set the property in > READY :)


Ok, so what do you suggest for the closing problem? In which code is it actually a problem? I would say that udpsrc should just forget the socket once it has closed it, and it's an application problem if this causes problems.
Comment 5 Patricia Muscalu 2016-04-27 13:37:14 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> Yes, you shouldn't set the property in > READY :)

Yes, I know, but the code allows it :) See how the property setting is handled in fdsrc or filesrc for example.

> Ok, so what do you suggest for the closing problem? In which code is it
> actually a problem? I would say that udpsrc should just forget the socket
> once it has closed it, and it's an application problem if this causes
> problems.

I do not know. It seems like external sockets should be handled outside the udpsrc. Just a spontaneous comment.
Comment 6 Sebastian Dröge (slomo) 2016-04-27 13:40:59 UTC
I would say udpsrc should just fail properly in this scenario. And gst-rtsp-server and other code should set close-socket=FALSE if they want to keep using sockets independent of the udpsrc states.
Comment 7 Patricia Muscalu 2016-04-28 06:16:45 UTC
Ok Sebastian. Thanks.
Comment 8 Sebastian Dröge (slomo) 2016-04-28 06:19:54 UTC
Do you want to prepare a patch for that?
Comment 9 Patricia Muscalu 2016-04-28 06:26:32 UTC
Yes, I'll try to solve the problem in the gst-rtsp-server code.
What about thread safety in udpsrc and setting properties in proper states?
Comment 10 Sebastian Dröge (slomo) 2016-04-28 06:50:44 UTC
An approach like in filesrc seems useful here too, yes
Comment 11 Göran Jönsson 2016-11-21 12:33:36 UTC
Created attachment 340416 [details] [review]
gst-rtsp-server patch

Hi.
I have taken over this fro Patricia since she got other things to do.
Please find attached patch
Comment 12 Sebastian Dröge (slomo) 2016-11-21 15:53:53 UTC
Review of attachment 340416 [details] [review]:

::: gst/rtsp-server/rtsp-stream.c
@@ +1178,3 @@
 
+  g_object_set (G_OBJECT (udpsrc_out[0]), "close-socket", FALSE, NULL);
+  g_object_set (G_OBJECT (udpsrc_out[1]), "close-socket", FALSE, NULL);

But who is closing the socket, and where? :) Please add a comment and/or mention this in the commit message. Also why the change is done
Comment 13 Göran Jönsson 2016-11-22 08:40:18 UTC
Created attachment 340492 [details] [review]
gst-rtsp-server patch

The background of this ticket is a left over from https://bugzilla.gnome.org/show_bug.cgi?id=759773

After we have started using the solution from above tickets we still get the 
"g_inet_socket_address_get_address: assertion 'G_IS_INET_SOCKET_ADDRESS (address)' failed"
print sometimes in our stab and stress tests sometimes. 

Patricia analyzed this and the reason for this print was as in the attached
unit test.
Comment 14 Sebastian Dröge (slomo) 2016-11-22 12:02:15 UTC
commit 335d279a96bc91fda43ee3e1028ee3c327e542ad
Author: Göran Jönsson <goranjn@axis.com>
Date:   Mon Nov 21 13:05:50 2016 +0100

    rtsp-stream: Set close-socket FALSE on UDP src:es
    
    With this RTSP server can use the sockets independent on the udpsrc
    state.
    When the udp src is finalized it will unref socket and when g_socket
    is finalized the socket will be closed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765673