Red Hat Bugzilla – Bug 220562
Update 2.10.4-7 breaks third party programs
Last modified: 2007-11-30 17:11:52 EST
Description of problem:
The fix of bug #218755 (which is inaccessible -- as well as all other recent
gtk2 bugs -- so I don't know what was going on there) introduced in
gtk2-2.10.4-7 causes CRITICAL errors (that can also mean app termination) in
programs that close the pixbuf loader themselves on gdk_pixbuf_loader_write()
1. No public method to check whether the loader was closed exists. What's
worse, signal "closed" is NOT emitted on this failure-induced close. The
behaviour change is not clearly associated with any public version change.
Therefore after getting FALSE from gdk_pixbuf_loader_write() applications can't
tell whether the pixbuf loader is closed or not.
2. Closing the loader anyway produces CRITICAL errors in gtk2 >= 2.10.4-7.
3. Assuming it was closed and just dereferencing it produces warnings in gtk2 <
Where 2 is what many programs do now because they have been getting the warnings
if they did 3.
Fedora/RHEL packages can in principle require/conflict with the appropriate
versions of Gtk+, but third party programs are busted.
Version-Release number of selected component (if applicable):
Steps to Reproduce:
1. Run for example gqview on a broken image (well, not every broken image leads
to this type of failure, I will attach some image if such a demonstration is
GdkPixbuf-CRITICAL **: gdk_pixbuf_loader_close: assertion `priv->closed ==
My personal opinion is that it has been broken for so long that it the broken
behaviour has become *the* API and instead of changing the behaviour, the
sentence about closing the loader should have been removed from the
documentation -- or at least gdk_pixbuf_loader_close() made idempotent.
In any case the fact the pixbuf is closed but "closed" is not emitted is a bug
of itself -- I can file a separate bugreport if it is considered a separate issue.
The bug that triggered this change was a case of a library ignoring the return
value of gdk_pixbuf_loader_write and feeding more garbage into the loader after it
already returned FALSE. This unfortunately triggered a crash inside one of the
So I thought it would be better to make write() behave as documented and close
the loader when it returns FALSE. This also shows that you are not correct, it
is in fact possible for a user of this api to know if the loader is closed -
simply pay attention to the return value of write(). Anyway, a further change
that I have already made in upstream cvs is to in fact make close() silently
return if the loader is already closed. I'll push that out as an FC6 update
I wasn't sure if it is a good idea to emit the closed signal when the loader
is closed due to error, but I can do that too.
(In reply to comment #1)
> This also shows that you are not correct, it
> is in fact possible for a user of this api to know if the loader is closed -
> simply pay attention to the return value of write().
No, I don't know because I cannot tell whether I use a version of Gtk+ that
actually closes it, or version that does not close it.
> Anyway, a further change
> that I have already made in upstream cvs is to in fact make close() silently
> return if the loader is already closed.
> I wasn't sure if it is a good idea to emit the closed signal when the loader
> is closed due to error, but I can do that too.
I cannot tell how other people write their programs, but if they use "closed", I
suppose they bound some clean-up action to it. For years, they have been
calling gdk_pixbuf_loader_close() themselves after failure -- because not doing
so used to lead to
GdkPixbuf-WARNING **: GdkPixbufLoader finalized without calling
gdk_pixbuf_loader_close() - this is not allowed. You must explicitly end the
data stream to the loader before dropping the last reference
-- and their clean up actions were always run because "closed" was always
emitted before finalization of the loader. Now the pixbuf loader closes itself
on failure, further gdk_pixbuf_loader_close() just returns (once it's fixed),
"closed" is not emitted and the clean-up action is not run.
On second thought, emitting "closed" in failure-incuded close makes it happen in
different time than it used to, so some assertions in its handler can fail. To
make it really backward compatible, one would have to add a flag to priv whether
the loader was closed *explicitly* by calling gdk_pixbuf_loader_close(). It
1. If it was not closed at all, behave normally and set closed and the new flag.
2. If it was closed implicitly, do nothing except emitting "closed" and setting
3. If it was closed explicitly, fail noisily.
So this also allows preservation of an analogue to
g_return_val_if_fail (priv->closed == FALSE, TRUE);
Of course, it is a matter of how you value backward compatibility...
Please try version thats in updates-testing now, and tell me if you see any
further problems with it. I'll leave the signal emission where it is now unless
it causes problems.
I can see only 2.10.4-9 in testing updates with an unrelated changelog
* Thu Dec 21 2006 Matthias Clasen <firstname.lastname@example.org> - 2.10.4-9
- Make update scripts handle slight variations in $host
and it behaves the same. I suppose I should wait for another testing update
(2.10.4-10?) to get to the mirrors.
OK, with gtk2-2.10.4-10.fc6 programs expecting both the old behaviour and the
behaviour specified in the documentation seem to work and print no warnings.