#560: WinGRASS not deleting temp ppm files from map display
------------------------------+---------------------------------------------
Reporter: isaacullah | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: Display | Version: 6.4.0 RCs
Keywords: v.digit ppm temp | Platform: MSWindows XP
Cpu: x86-32 |
------------------------------+---------------------------------------------
When v.digit loads, it creates it's own ppm and ppg files for the
background of the v.digit display (seperate from the ppm and ppg files
created for the regular tcltk map display). When one exits from v.digit,
those ppm and ppg stay in the .tmp directory of the mapset. When one exits
from grass, the ppm and ppg files for the map display are erased, but NOT
those created by the v.digit display. This leads to build up of files that
can eat harddrive memory. This problem was dectected on a Windows XP
machine, and has not been tested for on Linux or Mac.
#560: WinGRASS not deleting temp ppm files from map display
---------------------------+------------------------------------------------
Reporter: isaacullah | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: Display | Version: 6.4.0 RCs
Resolution: | Keywords: v.digit ppm temp
Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by glynn):
Replying to [comment:1 hamish]:
> the only thing I can think of is that the files are still in use, so can
not be swept up by clean_temp on exit. ?
AFAICT, clean_temp doesn't work (at all) on Windows. See the "TODO"s in
the code.
clean_temp only removes files if they are owned by the current user, and
either the PID is no longer in use or the file is more than 4 days old.
But it cannot determine the current user on Windows (no getuid()), so it
uses -1, but MSVCRT's stat() always reports zero for the st_uid field.
Also, the code which checks whether a given PID is still in use always
fails (which is handled the same as if the PID *is* in use).
If the current user check was skipped, it should at least delete files
more than 4 days old.
Implementing the ownership check would be non-trivial, due to the
complexity of the NT security model (e.g. a file can be owned by a group
rather than a user, there's a difference between owner and creator, a
"user" consists of both a domain and a username, etc).
this isn't just v.digit; just using the wxGUI layer manager to render
raster and vector maps is enough to leave many tmp*.ppm and tmp*.pgm files
behind.
upgrading to critical because over time many MB of .ppm files will pile up
in `C:\Documents and Settings\$USER\Local Settings\Temp` and eat all your
disk space.
Replying to [comment:8 hamish]:
> this isn't just v.digit; just using the wxGUI layer manager to render
raster and vector maps is enough to leave many tmp*.ppm and tmp*.pgm files
behind.
>
>
> upgrading to critical because over time many MB of .ppm files will pile
up in `C:\Documents and Settings\$USER\Local Settings\Temp` and eat all
your disk space.
>
in Win7 the left files are in C:\Users\$USER\AppData\Local\Temp
Replying to [comment:9 hellik]:
> Replying to [comment:8 hamish]:
> > this isn't just v.digit; just using the wxGUI layer manager to render
raster and vector maps is enough to leave many tmp*.ppm and tmp*.pgm files
behind.
> >
> >
> > upgrading to critical because over time many MB of .ppm files will
pile up in `C:\Documents and Settings\$USER\Local Settings\Temp` and eat
all your disk space.
> >
>
> in Win7 the left files are in C:\Users\$USER\AppData\Local\Temp
These files should be stored in `.tmp` directory localed in current GRASS
mapset I would say.
> These files should be stored in `.tmp` directory localed in current
GRASS mapset I would say.
The the mapset's .tmp directory is meant for files which will be moved to
elsewhere within the mapset directory once they have been written (e.g.
the cell/fcell files). Files cannot be rename()d across filesystem
(partition) boundaries, so the temporary file must be on the same
filesystem as the intended destination. The mapset's .tmp directory can be
assumed to be on the same filesystem as the rest of the mapset.
Files which don't have this requirement should either use the OS'
temporary directory, or a configurable directory. Access to the temporary
directory may be significantly faster than to the mapset directory,
particularly if the mapset is on a network filesystem.
Replying to [comment:11 glynn]:
> Files which don't have this requirement should either use the OS'
temporary directory, or a configurable directory. Access to the temporary
directory may be significantly faster than to the mapset directory,
particularly if the mapset is on a network filesystem.
So, all files created for wxGUI are standard temporary files without
special requirements and should be created by standard system/python way,
is that rigth?
Standard way is using
[http://docs.python.org/library/tempfile.html#tempfile.mkstemp
tempfile.mkstemp] and then delete the file (and of course, this has to be
ensured even when error/exception occurs). Files which was not opened
(files to be combined to resulting map) doesn't have to be closed. Only
files which was opened (like resulting map) is necessary to close (see
[http://www.logilab.org/blogentry/17873 this]).
Now, mkstemp is used for files which will be combined to resulting and
then os.remove (grass.try_remove) is called. So, I'm not sure where is the
problem (why temporary files are not removed).
> So, all files created for wxGUI are standard temporary files without
special requirements and should be created by standard system/python way,
is that rigth?
AFAIK, yes.
> Files which was not opened (files to be combined to resulting map)
doesn't have to be closed.
mkstemp returns an OS-level handle to the opened file. This should be
closed (with os.close()) if it is not required.
> Now, mkstemp is used for files which will be combined to resulting and
then os.remove (grass.try_remove) is called. So, I'm not sure where is the
problem (why temporary files are not removed).
On Windows, you can't remove a file if any process has it open.
Replying to [comment:13 glynn]:
> Replying to [comment:12 wenzeslaus]:
...
> > Now, mkstemp is used for files which will be combined to resulting and
then os.remove (grass.try_remove) is called. So, I'm not sure where is the
problem (why temporary files are not removed).
> On Windows, you can't remove a file if any process has it open.
Could the removal be put into the shutdown sequence? Perhaps by putting
the files
in question into a (file based) list?
Replying to [comment:14 neteler]:
> Could the removal be put into the shutdown sequence? Perhaps by putting
the files
> in question into a (file based) list?
the gui should be cleaning up after itself, but perhaps the wxGUI temp
.ppm, .pgm, and 0 byte no-extension versions of the same files could at
least be placed in a \grass-wxgui subdir of \Temp\, so at least they are
consolidated and easier to manually bulk clean up if grass is shutdown
uncleanly?
fwiw the tcl/tk gui in wingrass saves its .ppm files to $MAPSET/.tmp, and
succeeds in cleaning them up. It uses g.pnmcomp too, so I guess that
g.pnmcomp exiting without explicitly closing the image files is not the
reason for failure-to-delete?
Hamish
(argh trac is logging me off every 10 minutes today..)
the empty file versions may be related to #943, core/render.py creates
some temp files without extension, then creates some more in /tmp/ with
e.g. .ppm extension. Note that way cancels the benefit of using
tempfile.mkstemp(), since the temp file created is no longer created in a
secure way, and /tmp is a+w. (at least $MAPSET/.tmp/ is u+w only)
after the empty temp files, there are still .ppm files not being removed
too.