Bug 101173

Summary:	double locking issue on the session
Product:	Farstream	Reporter:	Fabrice Bellet <fabrice>
Component:	RTP Plugin	Assignee:	Olivier Crête <olivier.crete>
Status:	RESOLVED MOVED	QA Contact:
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	rtp: fix a double locking issue on the session rtp: fix a double locking issue on the session

Description Fabrice Bellet 2017-05-24 19:52:23 UTC

Created attachment 131487 [details] [review]
rtp: fix a double locking issue on the session

Another rare situation: fs_rtp_stream_add_substream_unlock() may be unable to unlock the session, previously locked by the parent function fs_rtp_session_new_recv_pad(), if stream->priv->session becomes null in the meanwhile.

The workaround for this, and it's again not very elegant, is to pass directly the session pointer to the function having to unlock it.

I don't have any gdb trace for this case.

Comment 1 Olivier Crête 2017-06-05 23:19:00 UTC

Comment on attachment 131487 [details] [review]
rtp: fix a double locking issue on the session

Review of attachment 131487 [details] [review]:
-----------------------------------------------------------------

Arg, I never though of this case.

::: gst/fsrtpconference/fs-rtp-stream.c
@@ -1025,5 @@
>      FsRtpSubStream *substream,
>      GError **error)
>  {
>    gboolean ret = TRUE;
> -  FsRtpSession *session = fs_rtp_stream_get_session (stream, error);

We still need the get_session() call as it's used as a guard against using a disposed stream.. So we sadly want something like 

FsRtpSession *mysession = fs_rtp_stream_get_session (stream, error);

if (!mysession) { UNLOCK (session); return FALSE; }
g_object_unref (mysession);

::: gst/fsrtpconference/fs-rtp-stream.h
@@ +120,1 @@
>      FsRtpSubStream *substream,

Can you put the session pointer not as the first paramter but as the second to last (just before the GError), the convention in GObject is that the "self" pointer is always the first parameter.

Comment 2 Fabrice Bellet 2017-06-13 13:14:24 UTC

Created attachment 131925 [details] [review]
rtp: fix a double locking issue on the session

This is an updated version of the patch with your suggestions included.

Comment 3 GitLab Migration User 2018-08-21 12:16:50 UTC

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/farstream/farstream/issues/13.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.