Bug 793863 – curlhttpsrc: deadlock in multi-instance scenario

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 793863 - curlhttpsrc: deadlock in multi-instance scenario


Summary:	curlhttpsrc: deadlock in multi-instance scenario


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-bad
Version:	git master
Hardware:	Other Linux

Importance:	Normal critical
Target Milestone:	1.14.1
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2018-02-27 02:21 UTC by Jun Xie
Modified:	2018-04-28 07:48 UTC

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

Attachments
easily reproduced with this test file (1.81 KB, text/x-csrc) 2018-02-27 02:22 UTC, Jun Xie		Details
fix deadlock issue (1.10 KB, patch) 2018-02-27 02:24 UTC, Jun Xie	committed	Details \| Review

Description Jun Xie 2018-02-27 02:21:43 UTC

1. in gst_curl_http_src_curl_multi_loop, if state is GSTCURL_MULTI_LOOP_STATE_REQUEST_REMOVAL, queue is not iterated at all, will deadlock here in multi-instance scenario
2. state will be deadlock in GSTCURL_MULTI_LOOP_STATE_REQUEST_REMOVAL even other instance is under running.

Comment 1 Jun Xie 2018-02-27 02:22:48 UTC

Created attachment 368985 [details]
easily reproduced with this test file

easily reproduced with this test file

Comment 2 Jun Xie 2018-02-27 02:24:51 UTC

Created attachment 368986 [details] [review]
fix deadlock issue

Comment 3 Jun Xie 2018-02-27 02:25:54 UTC

hi, Sam Hurst, would you please look into it？

Comment 4 Sam Hurst 2018-03-06 14:25:30 UTC

Review of attachment 368986 [details] [review]:

Your change looks good to me, and I've merged it on the GitHub project. If there is a GStreamer maintainer who wants to double check it and can push this to git, that would be great.

Comment 5 Jun Xie 2018-03-07 01:17:42 UTC

Thanks for reviewing, Sam.

Comment 6 Jun Xie 2018-03-29 03:53:35 UTC

Since it has been merged in GitHub, please also merge it into GStreamer repo

Comment 7 Nicolas Dufresne (ndufresne) 2018-04-24 18:08:02 UTC

What does merged in github means ?

Comment 8 Nicolas Dufresne (ndufresne) 2018-04-24 18:18:01 UTC

Looks good otherwise.

Comment 9 Nicolas Dufresne (ndufresne) 2018-04-24 18:25:32 UTC

Thanks, I've only reworked a bit the comment, nothing else.

Author: Jun Xie <jun.xie@samsung.com>
Date:   Tue Feb 27 10:51:07 2018 +0800

    curlhttpsrc: deadlock in multi-instance scenario
    
    Fixed queue iterator issue and set context state to
    GSTCURL_MULTI_LOOP_STATE_RUNNING in case other
    instance are in running state.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=793863

Comment 10 Nicolas Dufresne (ndufresne) 2018-04-24 18:40:22 UTC

Seems like a candidate for 1.14 backport, what do you think ?