generated temp files are not deleted

[graemeNPS]

We use imagemagick to extract a jpg preview and jpg thumbnail from a wide variety of image file types, using a standard convert command to perform the initial extraction from (eg) a TIF file to a jpg preview, and then a resize on the jpg to get a thumbnail. We have a dll that statically links IM code based on the ImageMagick-6.5.9 release. We build an 8-bit version as it was faster for most of the files we use.

Some files take a long time to process, and we have a time-out in our workflow. If the IM processing get stopped/aborted for any reason, such as a time-out, we end up with temporary files with names like magick-sDHFMNhK in the temp file location (eg in C:/windows/temp). In a particular case, processing a TIFF file with a size of 920 KB (942,956 bytes) generates a temp file of 4.19 GB (4,499,644,032 bytes). I see a similar phenomenon with large PSD files.
Is there any way of our calling code knowing these files are being created, to allow us to delete them if we detect an error condition? If we knew the filename, we could possibly remove(filename) in the destructor of the object that calls IM.

Thanks for any help,
Graeme

[magick]

ImageMagick requires temporary files when caching pixels to disk or when creating intermediate files for delegate programs. All the temporary filenames begin with magick-. ImageMagick includes signal handlers so that it can clean-up any temporary files before it exits. The actual temporary file clean up method is magick/resource.c/AsynchronousResourceComponentTerminus. In some cases, ImageMagick is not given the opportunity to call its clean-up code to properly delete temporary files. In this case you need to remove them manually. You could run a job each night to delete magick-* from the temporary directory. A better solution might be to create a temporary folder for each call to ImageMagick and point ImageMagick to it (e.g MAGICK_TMPDIR=/data/magick/abacadaba). That way when the process is complete, simply remove that folder to ensure temporary files are removed without interfering with other ImageMagick processes.

[graemeNPS]

Are you suggesting I amend our convert command for each call to IM (assuming /someWindowsTempPath/randomChars/ exists) to include a -define registry:temporary-path option?
"convert -define registry:temporary-path=/someWindowTemppath/randomChars/ ..."
or is there another way of setting MAGICK_TEMPORARY_PATH from Windows.

I see there is a -define option and a -set option. Any difference/preference?

Our current convert command, passed as an array to
status=MagickCore::ConvertImageCommand(image_info,argc,argv,(char **) NULL,&exception);
looks like this:
convert -auto-orient -size 49608x22676 \\somepath\XX_tiff_files\somefile-3-LrgLnd-Flitter.tif[0]
-alpha Off -resize 512x512 -profile C:\Program Files\IPieces\sRGB.icm +profile "*" someotherpath\Temp\4200viewx.jpg

[magick]

Yes, use -define registry:temporary-path. Or perhaps when your process times out, you send a soft signal that ImageMagick can intercept under Windows and it will clean up its own temporary files.

[graemeNPS]

Hi, this command works, passed as array argv(as shown in earlier post), so that I know where the temp files are:
[convert]
[-define]
[registry:temporary-path=C:\DOCUME~1\someuser\LOCALS~1\Temp\NPS_IM_0]
[-auto-orient]
[-size]
[21260x12553]
[C:\Documents and Settings\someuser\My Documents\somepath\IG_tiff_files\WP068.TIF[0]]
[-alpha]
[Off]
[-resize]
[512x512]
[-profile]
[C:\Program Files\IPieces\sRGB.icm]
[+profile]
["*"]

However, we also have code like the following code snippet and we have a TIFF that fails (throws an exception) for both
pImage->ping(filename);
and
pImage = new Magick::Image(szOrgFilePath);

and that leaves 0-byte magick-XXXXXX file in the temp directory that are locked. I have to reboot to get rid of them.
Any thoughts on how to handle this? Note this code is run before we call
status=MagickCore::ConvertImageCommand(image_info,argc,argv,(char **) NULL,&exception);
so the temporary directory has not been changed yet. Any thoughts on how to change it for this case?
(Sometimes this particular file has worked, and it creates a magick-xxx temp file of about 4+GB.)

//BEGIN snippet
Magick::InitializeMagick(NULL);
Magick::Image * pImage = NULL;
try
{
pImage = new Magick::Image();
pImage->ping(szOrgFilePath);
}
catch(...)
{
// Sometimes loading a file with ping fails! So if that happens we'll try again with the full method.
try
{
logger(fname, LOG_INFO, "this file [%s] could not be opened with ping(..)", szOrgFilePath);
// Clean up the image just in case.
if(NULL != pImage)
{
delete pImage;
pImage = NULL;
}

pImage = new Magick::Image(szOrgFilePath);
}
catch(...)
{
// If this method also fails, get out of here.
logger(fname, LOG_INFO, "this file [%s] can not be handled by the ImageMagick IPiece", szOrgFilePath);

// Clean up the image just incase.
if(NULL != pImage)
{
delete pImage;
pImage = NULL;
}

return "";
}

}
//END snippet

[magick]

It would be helpful if we can reproduce the problem. Can you post a URL to your image so we can download it and see where the exception is thrown.

[lxnt]

We have encountered the same problem.

Reproduction:

convert -limit time 1s -limit memory 100m -limit map 100m -resize 100000x100000 tl.jpg lt.jpg


Core issue: Temporary file names are stored in a splay_tree. When splay_tree is destroyed, first relinquish_key() is called, then relinquish_value().
For temporary files, both key and value point to the same string - the file name. On tree destruction, first relinquish_key() frees the string, then relinquish_value() tries to remove() the file.

When just-freed memory is not garbled by something else, like memory manager internals, all works. When it is - remove() fails.


Solution: do not rely on order in which relinquish functions are called.

Patch:

Code: Select all

--- pristine/imagemagick-6.5.7.8/magick/resource.c 2009-10-26 16:52:10.000000000 +0300
+++ libm/imagemagick-6.5.7.8/magick/resource.c 2010-09-28 19:18:39.000000000 +0400
@@ -329,6 +329,7 @@
 static void *DestroyTemporaryResources(void *temporary_resource)
 {
   (void) remove((char *) temporary_resource);
+ RelinquishMagickMemory(temporary_resource);
   return((void *) NULL);
 }
 
@@ -474,10 +475,10 @@
   (void) LockSemaphoreInfo(resource_semaphore);
   if (temporary_resources == (SplayTreeInfo *) NULL)
     temporary_resources=NewSplayTree(CompareSplayTreeString,
- RelinquishMagickMemory,DestroyTemporaryResources);
+ DestroyTemporaryResources, NULL);
   (void) UnlockSemaphoreInfo(resource_semaphore);
   resource=ConstantString(path);
- (void) AddValueToSplayTree(temporary_resources,resource,resource);
+ (void) AddValueToSplayTree(temporary_resources,resource,NULL);
   return(file);
 }


[magick]

Good catch. We'll apply your patch to ImageMagick 6.6.4-8 Beta by sometime tomorrow. Thanks.