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 334943 - make check FAIL: threadpool-test
make check FAIL: threadpool-test
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gthread
2.10.x
Other All
: Normal normal
: ---
Assigned To: Martyn Russell
gtkdev
Depends on:
Blocks:
 
 
Reported: 2006-03-18 00:04 UTC by sam
Modified: 2011-02-18 15:49 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
a patch (758 bytes, patch)
2006-03-20 14:09 UTC, Matthias Clasen
none Details | Review
Improves on first patch (3.59 KB, patch)
2006-03-23 16:26 UTC, Martyn Russell
committed Details | Review

Description sam 2006-03-18 00:04:21 UTC
Please describe the problem:
build on sparc solaris 9 gcc version 4.0.2 
make finishes 
make check  
** ERROR **: file threadpool-test.c: line 124 (test_thread_sort_entry_func): 
assertion failed: (last_thread_id <= thread_id) 
aborting... 
FAIL: threadpool-test 
... 
===================================================================== 
1 of 47 tests failed 
Please report to http://bugzilla.gnome.org/enter_bug.cgi?product=glib 
===================================================================== 
make[4]: *** [check-TESTS] Error 1 
 

Steps to reproduce:
1. make check 
2.  
3.  
 

Actual results:
** ERROR **: file threadpool-test.c: line 124 (test_thread_sort_entry_func): 
assertion failed: (last_thread_id <= thread_id) 
aborting... 
FAIL: threadpool-test 
... 
===================================================================== 
1 of 47 tests failed 
Please report to http://bugzilla.gnome.org/enter_bug.cgi?product=glib 
===================================================================== 
make[4]: *** [check-TESTS] Error 1 
 

Expected results:
no failures  

Does this happen every time?
yes 

Other information:
Comment 1 Matthias Clasen 2006-03-20 04:03:53 UTC
Hmm, I have seen this too. And I think the logic behind last_failed is flawed.
I think it is possible for more than one item to appear out-of-order, if the
queue runs empty before the next item is pushed.

Martyn ?
Comment 2 Matthias Clasen 2006-03-20 14:07:31 UTC
To be more precise, I have seen this fail in our build system, on >=8-way machines. Here is the patch I used to make it pass.
Comment 3 Matthias Clasen 2006-03-20 14:09:01 UTC
Created attachment 61618 [details] [review]
a patch
Comment 4 Matthias Clasen 2006-03-20 14:13:36 UTC
As  rambokid points out, even with this patch, the out-of-order detection
logic is still vulnerable to scheduler races:

<rambokid> mclasen: that is still scheduler-racy though. the way the test is setup, it will only be non-racy if exactly 1 worker thread is used.
<rambokid> mclasen: simply because before the first G_LOCK() the scheduler can decide to exec the threads in any order
<mclasen> rambokid: but if all queuing happens before any work is done, you will never run into out-of-order items, or am I missing something ?
<rambokid> mclasen: yeah, you're missing the esystem scheduler. if you execute multiple threads in parallel, they can be worked on in any order. so if you start threads 1,2,3,4,5,6,7,8, the system scheduler can decide to work on them in order 8,7,6,5,4,3,2,1, so they'd acquirte the first G_LOCK in the test funciton in wrong order, and the assertion logic gets triggered.
<mclasen> ah, thats true
Comment 5 Martyn Russell 2006-03-21 22:23:16 UTC
Since we have no way to know which order the system scheduler will decide to exec threads and hence which G_LOCK will get in first, I see only two options:

1. We remove this particualr test.
2. We allow an acceptable level of error.

I think we should probably remove the test.
Comment 6 Matthias Clasen 2006-03-22 13:56:23 UTC
Fine with me
Comment 7 Tim Janik 2006-03-22 14:27:19 UTC
not fine with me though... ;)
i think the basic order testing that is accomplished does make sense. it just can't be properly tested in a multi-threaded scenario.
so simply limit the number of worker threads to 1 for the duration of the test, that way, the ordering is still checked.
Comment 8 Martyn Russell 2006-03-23 16:26:53 UTC
Created attachment 61853 [details] [review]
Improves on first patch

This patch basically does what Matthias' patch did except I have added to the documentation, added some comments in the code and removed the debugging so 'make check' is less noisy.

If you guys are happy with the documentation addition I will go ahead and commit it.
Comment 9 Matthias Clasen 2006-03-23 18:02:16 UTC
Looks good to me. 
Comment 10 Martyn Russell 2006-03-24 15:23:36 UTC
Committed.