Fwd: GUI signals from a thread (GTK+-3.10)

[Andrew Potter <agpotter gmail com>]

Oops, missed the list on my reply...

---------- Forwarded message ----------
From: Andrew Potter <agpotter gmail com>
Date: Sat, Feb 15, 2014 at 9:41 AM
Subject: Re: GUI signals from a thread (GTK+-3.10)
To: Valentin But <valentin but eesti ee>


On Fri, Feb 14, 2014 at 9:25 PM, Valentin But <valentin but eesti ee> wrote:
> I've changed the code, now it's a perfect GTK+ threading example with
> a progress bar! Will somebody add it to gtk3-demo and documentation?

Its good to hear you're interested in contributing. But it is
dangerous to send a message to a public mailing list claiming
perfection! Somebody may feel like nitpicking. For instance.. ;)

You've chosen to allocate your THREAD_PROGRESS_DATA on the stack, and
have passed the address of the stack variable to g_idle_add().
Presumably you don't want to allocate and free a ton of
progress_datas, which is understandable. But there are two problems
with your approach which are fundamental to safely implementing
threaded programs.

Problem 1) Passing stack variables from a worker thread is dangerous.
Consider this sequence of events:
### GTK+ Main Loop ###
calls thread_progress()...
### Thread ###
The loop ends, and g_source_remove() is called. But thread_progress is
in the middle of executing, and g_source_remove() isn't going to do
anything about that.
thread_func() leaves scope, and the address of progress_data is now invalid!
### GTK+ Main Loop ###
Tries to dereference progress_data. Who knows what it finds.
If the thread is destroyed by this point, then you may even segfault.

If you want to pass stack variables around via pointer, you must be
absolutely sure the lifespan of the variable is longer than the
functions that can access them.

In this case, consider making THREAD_PROGRESS_DATA a member of your
CallbackObject. This way its data lifetime exceeds that of the thread
for sure.

Problem 2) Reading and writing variables across threads is dangerous.
Most variables on most architectures are not "atomically" updated.
Indeed, you should never assume atomicity unless you are leveraging
language or library features that explicitly give it. In this case,
the gdouble x variable may only be half updated when it is read.
Admitably this isn't a critical error in this case, but if someone
were to take the same approach with updating a pointer they could get
into real trouble. As such it shouldn't be presented as example code.

The solution in this case would be to include and use a GMutex to
guard access to all variables that multiple threads are touching.

You have a similar problem with the way you have implemented aborting
the thread. For this, a GCancellable provides a safe method. Your
button can call g_cancellable_cancel() and inside the thread you can
check g_cancellable_is_canceled().

The above are critical issues. As far as meaningless nitpicking,
returning G_SOURCE_CONTINUE in thread_progress() describes its
behavior more explicitly. You can also annotate messagebox() with the
G_GNUC_PRINTF(2, 3) macro to enable compile-time format checking for
calls to that function.

These changes would make your program much closer to perfect I think.

Cheers!