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 737999 - systemclock: multi-thread entry status issue
systemclock: multi-thread entry status issue
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.4.3
Other Linux
: Normal normal
: 1.4.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-06 12:26 UTC by Nicolas Huet
Modified: 2014-10-24 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
test file to reproduce the issue (2.99 KB, text/x-csrc)
2014-10-06 12:26 UTC, Nicolas Huet 		  Details
fix multi-thread entry status issue (2.58 KB, patch)
2014-10-06 12:27 UTC, Nicolas Huet 		committed Details | Review

Description Nicolas Huet 2014-10-06 12:26:17 UTC 
Created attachment 287844 [details]
test file to reproduce the issue

Running two threads, one executing the timer and one unscheduling it, the
unscheduled status set by the second thread is sometimes overwritten by the
first one.

I have attached a C test file to reproduce the issue

Here is the log I have with the small test application:
** Message: set timer
** Message: task GST_CLOCK_EARLY
** Message: unschedule current timer
** Message: task GST_CLOCK_UNSCHEDULED
** Message: set timer
** Message: task GST_CLOCK_EARLY
** Message: unschedule current timer
** Message: task GST_CLOCK_OK
** Message: task GST_CLOCK_OK
** Message: task GST_CLOCK_OK

Once the status is overwritten, I do not receive anymore the GST_CLOCK_UNSCHEDULED.

I have attached the patch that fix this issue.
Comment 1 Nicolas Huet 2014-10-06 12:27:05 UTC 
Created attachment 287845 [details] [review]
fix multi-thread entry status issue
Comment 2 Sebastian Dröge (slomo) 2014-10-06 12:41:12 UTC 
Review of attachment 287845 [details] [review]:

::: gst/gstsystemclock.c
@@ -739,3 @@
           /* timeout, this is fine, we can report success now */
-          status = GST_CLOCK_OK;
-          SET_ENTRY_STATUS (entry, status);

You're not setting the status at all anymore

@@ -768,3 @@
-      status = GST_CLOCK_EARLY;
-
-    SET_ENTRY_STATUS (entry, status);

And here
Comment 3 Nicolas Huet 2014-10-06 12:56:40 UTC 
Review of attachment 287845 [details] [review]:

::: gst/gstsystemclock.c
@@ -739,3 @@
           /* timeout, this is fine, we can report success now */
-          status = GST_CLOCK_OK;
-          SET_ENTRY_STATUS (entry, status);

I set the status using CAS_ENTRY_STATUS.
If the status has been modified by another thread, I do not modify it.
Comment 4 Sebastian Dröge (slomo) 2014-10-06 13:01:56 UTC 
Oh I missed that, sorry. Then the patch seems fine, thanks!

commit 00bcd94723f565c087ce559ab32cc49a74c30a8f
Author: Nicolas Huet <nicolas.huet@parrot.com>
Date:   Mon Oct 6 13:38:21 2014 +0200

    systemclock: fix multi-thread entry status issue
    
    Running two threads, one executing the timer and one unscheduling it, the
    unscheduled status set by the second thread is sometimes overwritten by the
    first one.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=737999