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.
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.
(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.
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...
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 <mclasen> - 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.