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 660635 - Deprecate g_thread_foreach
Deprecate g_thread_foreach
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gthread
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 660747
 
 
Reported: 2011-10-01 16:11 UTC by Matthias Clasen
Modified: 2011-10-03 05:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Deprecate g_thread_foreach (2.21 KB, patch)
2011-10-01 16:11 UTC, Matthias Clasen
none Details | Review
Drop g_thread_all_threads (10.85 KB, patch)
2011-10-01 17:37 UTC, Matthias Clasen
none Details | Review
Move g_thread_foreach to deprecated/ (3.42 KB, patch)
2011-10-01 17:46 UTC, Matthias Clasen
none Details | Review
Drop use of g_thread_foreach in tests (2.18 KB, patch)
2011-10-01 18:28 UTC, Matthias Clasen
none Details | Review
Whitespace fix (2.12 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
committed Details | Review
Rework the way GStaticPrivate data is freed (8.37 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
committed Details | Review
Add new thread creation API (11.51 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
none Details | Review
Make thread names useful in a debugger (3.26 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
none Details | Review
Don't put named threads on the global list (3.36 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
none Details | Review
Deprecate g_thread_foreach (2.70 KB, patch)
2011-10-02 14:05 UTC, Matthias Clasen
none Details | Review
Add new thread creation API (11.51 KB, patch)
2011-10-02 23:52 UTC, Matthias Clasen
none Details | Review
Make thread names useful in a debugger (3.26 KB, patch)
2011-10-02 23:52 UTC, Matthias Clasen
none Details | Review
Don't put threads created with g_thread_new() on the list (7.07 KB, patch)
2011-10-02 23:52 UTC, Matthias Clasen
none Details | Review
Deprecate g_thread_foreach (2.70 KB, patch)
2011-10-02 23:52 UTC, Matthias Clasen
none Details | Review
Deprecate GStaticPrivate (26.34 KB, patch)
2011-10-03 01:21 UTC, Matthias Clasen
none Details | Review

Description Matthias Clasen 2011-10-01 16:11:12 UTC
It is really not a very useful function, since there is virtually
nothing you can do with a GThread*, and implementing it requires
us to keep a list of threads around, and complicates things quite
a bit.
Comment 1 Matthias Clasen 2011-10-01 16:11:15 UTC
Created attachment 197964 [details] [review]
Deprecate g_thread_foreach
Comment 2 Matthias Clasen 2011-10-01 17:37:16 UTC
Created attachment 197966 [details] [review]
Drop g_thread_all_threads

This drops quite a bit of overhead in maintaining this list.

To avoid iterating threads in g_static_private_free(), defer freeing
the per-thread data to thread exit. The one complication here is
that it is possible for the static private index to be reused while
'old' data is still around. To deal with that case, store the 'owner'
with each per-thread data node, and free old data in
g_static_private_get() if the owner doesn't match. The remaining
possibility that a private index could be reused by a GStaticPrivate
with the same address is sufficiently unlikely that we can probably
ignore it.

With this change, per-thread data is now truly private again,
and we can drop the lock for it as well.
Comment 3 Matthias Clasen 2011-10-01 17:46:11 UTC
Created attachment 197970 [details] [review]
Move g_thread_foreach to deprecated/

At the same time, add a warning when this function is called,
since it does not do anything anymore.
Comment 4 Matthias Clasen 2011-10-01 18:28:40 UTC
Created attachment 197973 [details] [review]
Drop use of g_thread_foreach in tests

This was not reliable anyway, since we have private threads started
by GDBus and others now.
Comment 5 Allison Karlitskaya (desrt) 2011-10-02 04:10:40 UTC
So I had a thought about how we can fix this without breaking API in any way:

In bug 580505 we are looking for a way to introduce an API to set thread names.  I commented that we could do it by deprecating g_thread_create{,_full} and introducing g_thread_new{,_full}.

We could clarify the semantics of g_thread_foreach() as being "iterates over all threads created with g_thread_create()" (exactly as it is now) and make it so that g_thread_new() doesn't work with g_thread_foreach().  That would allow us to avoid the extra bookkeeping, except for in the deprecated case.
Comment 6 Matthias Clasen 2011-10-02 04:23:32 UTC
(In reply to comment #5)
> So I had a thought about how we can fix this without breaking API in any way:
> 
> In bug 580505 we are looking for a way to introduce an API to set thread names.
>  I commented that we could do it by deprecating g_thread_create{,_full} and
> introducing g_thread_new{,_full}.
> 
> We could clarify the semantics of g_thread_foreach() as being "iterates over
> all threads created with g_thread_create()" (exactly as it is now) and make it
> so that g_thread_new() doesn't work with g_thread_foreach().  That would allow
> us to avoid the extra bookkeeping, except for in the deprecated case.

Sure, that would be a possibility.
Comment 7 Matthias Clasen 2011-10-02 14:05:22 UTC
Created attachment 198008 [details] [review]
Whitespace fix
Comment 8 Matthias Clasen 2011-10-02 14:05:26 UTC
Created attachment 198009 [details] [review]
Rework the way GStaticPrivate data is freed

To avoid iterating threads in g_static_private_free(), defer freeing
the per-thread data to thread exit. The one complication here is
that it is possible for the static private index to be reused while
'old' data is still around. To deal with that case, store the 'owner'
with each per-thread data node, and free old data in
g_static_private_get() if the owner doesn't match. The remaining
possibility that a private index could be reused by a GStaticPrivate
with the same address is sufficiently unlikely that we can probably
ignore it.

With this change, per-thread data is now truly private again,
and we can drop the lock for it as well.
Comment 9 Matthias Clasen 2011-10-02 14:05:31 UTC
Created attachment 198010 [details] [review]
Add new thread creation API

Deprecate both g_thread_create functions and add
g_thread_new() and g_thread_new_full(). The new functions
expect a name for the thread.

Change GThreadPool, GMainContext and GDBus to create named threads.
Comment 10 Matthias Clasen 2011-10-02 14:05:34 UTC
Created attachment 198011 [details] [review]
Make thread names useful in a debugger

Associate the name with the system thread, so that debuggers
can see it. This is currently only implemented for Linux, using
prctl.
Comment 11 Matthias Clasen 2011-10-02 14:05:41 UTC
Created attachment 198012 [details] [review]
Don't put named threads on the global list

This lets us avoid the overhead of maintaining the global
list in the non-deprecated case.
Comment 12 Matthias Clasen 2011-10-02 14:05:45 UTC
Created attachment 198013 [details] [review]
Deprecate g_thread_foreach

It is really not a very useful function, since there is virtually
nothing you can do with a GThread*, and implementing it requires
us to keep a list of threads around, and complicates things quite
a bit.
Comment 13 Matthias Clasen 2011-10-02 14:09:36 UTC
Here is the other approach.

I think it looks fine; two things I don't like about it are

a) we keep all the code for dealing with the thread list in gthread.c

b) thread->name plays a double role here; maybe we need to bite the bullet and
add a separate 'is_in_list' field.
Comment 14 Matthias Clasen 2011-10-02 23:52:16 UTC
Created attachment 198043 [details] [review]
Add new thread creation API

Deprecate both g_thread_create functions and add
g_thread_new() and g_thread_new_full(). The new functions
expect a name for the thread.

Change GThreadPool, GMainContext and GDBus to create named threads.
Comment 15 Matthias Clasen 2011-10-02 23:52:23 UTC
Created attachment 198044 [details] [review]
Make thread names useful in a debugger

Associate the name with the system thread, so that debuggers
can see it. This is currently only implemented for Linux, using
prctl.
Comment 16 Matthias Clasen 2011-10-02 23:52:26 UTC
Created attachment 198045 [details] [review]
Don't put threads created with g_thread_new() on the list

This lets us avoid the overhead of maintaining the global
list in the non-deprecated case.
Comment 17 Matthias Clasen 2011-10-02 23:52:29 UTC
Created attachment 198046 [details] [review]
Deprecate g_thread_foreach

It is really not a very useful function, since there is virtually
nothing you can do with a GThread*, and implementing it requires
us to keep a list of threads around, and complicates things quite
a bit.
Comment 18 Matthias Clasen 2011-10-03 01:21:26 UTC
Created attachment 198052 [details] [review]
Deprecate GStaticPrivate

Move GStaticPrivate, g_thread_foreach and all related functions
to gthread-deprecated.c. We introduce some internal API to make
this possible.
Comment 19 Matthias Clasen 2011-10-03 05:05:49 UTC
All done now