It took me a day and a half, on and off, to find this

[ewx]

--- imagemagick-5.5.7.9.old/magick/blob.c       2003-05-23 19:26:28.000000000 +0100
+++ imagemagick-5.5.7.9/magick/blob.c   2003-12-31 19:25:41.000000000 +0000
@@ -418,7 +418,10 @@
   image->blob->eof=False;
   image->blob->status=status < 0;
   if (image->blob->exempt)
+  {
+    DetachBlob(image->blob);
     return;
+  }
   switch (image->blob->type)
   {
     case UndefinedStream:

...in my defence, I thought the bug was in a different place entirely for much of it.

Comments

[bellinghman.livejournal.com]

Hmmm. Must patch Monday.

I assume you've reported this?

I assume you've reported this?

[ewx.livejournal.com]

Yes, including upstream (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=225538), not that I've seen any evidence anyone is paying attention l-(

[songster.livejournal.com]

Any chance of explaining to the interested clueless what's wrong with the above?

[bellinghman.livejournal.com]

Basically, it wasn't detaching the image data when doing the early return. Without checking the source (and I've not got that code here, only on my PC at work), I assume that it means that a large chunk of data was left sticking to the image structure when it shouldn't have.

Later code would then have found the internals in the wrong state.

To a large extent, the library is prone to this sort of thing, being written in C while it really cries out to be in C++. Sadly, it's way too big to be easily converted. However, it's pretty damned good despite that.

[ewx.livejournal.com]

In the confusing world of imagemagick, a blob actually means a file, and the problem arises from CloseBlob being called twice, once to intentionally close the file and once when the image is destroyed, by which time the file happens to have been closed by its real owner (and CloseBlob always calls ferror on the file).

[bellinghman.livejournal.com]

As I recall (and I've not delved in the internals recently) the 'blob' terminology is because it's not necessarily a file that's being used. It can be an in-memory data chunk.

This is where C++ virtual functions and polymorphic types would make things so much simpler. Think of the 'blob' being like a std::istream, and it could be a file, a socket, a memory buffer, a database record, something that generates data as required ... This is prety much how the ImageMagick design is done, but because it's trying to work on anything with a half-way decent C compiler rather than requiring a compliant C++ compiler, it's much gnarlier.

[ewx.livejournal.com]

I called them files because the relevant thing that happens to them is that they are read from and writen to. I think it's perfectly possible to do it cleanly (or at least, much more cleanly than imagemagick does it) in C - function pointers rather than big switch statements for instance.