Bug 220562 - Update 2.10.4-7 breaks third party programs
Update 2.10.4-7 breaks third party programs
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: gtk2 (Show other bugs)
6
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2006-12-21 21:35 EST by David Nečas
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-09 23:31:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description David Nečas 2006-12-21 21:35:18 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()
failure.

Now:

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 <
2.10.4-7.

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):
2.10.4-7

How reproducible:
Always

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
necessary).
  
Actual results:
It prints
GdkPixbuf-CRITICAL **: gdk_pixbuf_loader_close: assertion `priv->closed ==
FALSE' failed

Expected results:
No warnings.

Additional info:
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.
Comment 1 Matthias Clasen 2006-12-21 22:01:02 EST
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
laoders. 

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
tonight.

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.
Comment 2 David Nečas 2006-12-22 03:26:37 EST
(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.

Thanks.

> 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.
Comment 3 David Nečas 2006-12-22 03:36:54 EST
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
could then:

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
the flag.

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...
Comment 4 Matthias Clasen 2006-12-22 08:55:12 EST
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.
Comment 5 David Nečas 2006-12-22 13:41:09 EST
I can see only 2.10.4-9 in testing updates with an unrelated changelog

* Thu Dec 21 2006 Matthias Clasen <mclasen@redhat.com> - 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.
Comment 6 David Nečas 2006-12-23 09:57:02 EST
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.

Note You need to log in before you can comment on or make changes to this bug.