Opened 6 years ago

Closed 6 years ago

#3635 closed defect (fixed)

Suspicious use and cleanup of the mapset tempoary directory

Reported by: wenzeslaus Owned by: wenzeslaus
Priority: normal Milestone: 7.8.0
Component: Startup Version: svn-trunk
Keywords: init grass.py clean_temp .tmp temp g.mapset wxGUI rendering Cc:
CPU: Unspecified Platform: All

Description

Similarly to wrong lockfile being removed in #3631, grass.py seems to clean and remove temporary directory associated with a mapset (during one startup procedure and during shutdown). However, when mapset is switched during the session, the clean up still happens in the initial mapset.

The clean up seems to be introduced in r54467 (before 7.0). Note that location here means full path to the mapset and the variable is not changed after initialization to the "start time" mapset. At the same time $GISBASE/etc/clean_temp is being called. Why clean_temp is not sufficient?

A reason I can see is that there seems to be a problem with GUI which continues to use the old mapset even after the switch for rendering (PPM and vector legend). The comment in grass.py indeed says "remove GUI session files from .tmp". Shouldn't it use the system temporary directory or always get the current mapset?

Consequently, after turning of GRASS GIS, the initial mapset's .tmp is removed completely while the current mapset's .tmp is cleaned in a sophisticated way using clean_temp. Then I would say the GUI needs to make sure to use the current mapset or not use mapset's temporary directory at all and that grass.py should only do cleaning using clean_temp. Does this make sense or am I missing something?

Change History (7)

comment:1 by wenzeslaus, 6 years ago

Owner: changed from grass-dev@… to wenzeslaus

The commit message for r54467 does not specify why the change - addition of os.path.join(location, ".tmp") - was needed and what is its relation to clean_temp. (Note that "location" means full path to mapset; same below.)

Similar line is cleanup_dir(os.path.join(location, ".tmp")) with comment "remove GUI session files from .tmp" which is called on MS Windows for GUI startup suggests some cleaning after GUI. However, it comes from the original translation of the startup script to Python (r37863). It originated from r21048 which was working around some bug related to Tcl/Tk GUI.

If GUI works as expected after #3637, the GUI files should disappear from the mapset tmp dir because all will be in the system tmp dir, then the line from r37863 won't be needed if its comment refers to rendered files.

As for r54467, I don't see why clean_temp is not enough. So, unless I find something, e.g. that clean_temp is broken or somebody suggests why it is there, I'm going to remove this clean up and let grass.py rely on clean_temp.

Preferred milestone: 7.8 (not in Trac yet)

comment:2 by neteler, 6 years ago

Milestone: 7.6.07.8.0

comment:3 by wenzeslaus, 6 years ago

In 73336:

init: do not remove wrong mapset .tmp, rely on clean_temp (see #3635)

The previous behavior was deleting whole mapset temporary directory
of a mapset set at the startup. When started in mapset A, switched to B,
exited, whole .tmp of A was removed, B was cleaned by clean_temp.
Now cleaning of A is done during switching (as before) but it is not
touched again, so no running processed in a (new) session in A are
in denger of loosing temporary files. The mapset active at exit is
cleaning using clean_temp as before, but only by that.

comment:4 by wenzeslaus, 6 years ago

In 73337:

init: remove basically dead code to clean mapset .tmp in default startup (see #3635)

The default (fallback) startup contained code which was probably from the
original Python rewrite (r37863) and maybe originated from workaround in r21048.
The clean_temp tool is called and takes care of cleaning, so this should not
be neccessary, not even for GUI (mentioned in the comment). GUI is not using
mapset temporary directory since r73334 (#3637).

comment:5 by wenzeslaus, 6 years ago

In 73338:

init: call clean_temp always at exit

This is has the same behaviour as before, i.e. clean_temp called at all exits
and it is similar to what was there before r73336, i.e. .tmp deleted at exit.
Additionally this calls clean_temp also in some error states when then
exist function calls in the call are not reached.

See #3635 for removal of Python-based clean up procedures (which should be covered by clean_temp).

comment:6 by wenzeslaus, 6 years ago

Now only the clean_temp tool is used. It is called through atexit.register(). clean_temp always cleans the current mapset (specified in an "rc file" pointed to by GISRC variable as for all GRASS modules).

What is now possible but was broken before:

  1. Start session I in mapset A.
  2. Switch session I to mapset B.
  3. Start session II in mapset A.
  4. Start processes in session II (mapset A).
  5. Exit session I (from mapset B).
  6. Running processes in session II (mapset A) still have their temporary files.

Before r73336, the processes in session II would fail because their temporary files were deleted. The positive aspect of this behavior was that the cleanup of the original mapset would take care of the files left behind by GUI which was putting some of its rendering files to the mapset temporary directory, but this is no longer needed after r73334 (#3637).

By relying only on clean_temp and not deleting whole .tmp manually (leaving aside the mapset switching problem), there are two changes in behavior:

  1. The .tmp directory and the $HOSTNAME subdirectory are never deleted once created. The content is cleaned by clean_temp which applies its rules rather than deleting all.
  2. When a process does not end in a standard way, e.g. by pressing Stop button in the wxGUI, the process may still show up when clean_temp tests if the process associated with the file by PID in the file name is still running (by sending the null signal; applies only to other than __MINGW32__ systems). Consequently, some files may be left behind when exiting the session (or changing mapset - that did not change). These files should be usually clean by the next session running in that mapset.

comment:7 by wenzeslaus, 6 years ago

Resolution: fixed
Status: newclosed

Closing as fixed, but considering changes related to locking (#3631), wxGUI rendering temporary files placement (#3637) and premature startup cleaning (#613, #1286), please test and please open new tickets if you encounter issues. Here are some commands to help with testing:

ls ~/grassdata/*/*/.tmp/$HOSTNAME
find ~/grassdata/*/*/.tmp/$HOSTNAME -not -empty -type d
find ~/grassdata/*/*/.tmp/$HOSTNAME -not -empty
ls -l ~/grassdata/*/*/.gislock
grass77 ~/grassdata/nc_spm_08_grass7/user1 --exec g.mapset user2
grass77 ~/grassdata/nc_spm_08_grass7/user1 --exec bash -c "g.mapset -c user2; g.mapset -c user3; g.mapset -c user4; sleep 10;"

Note: See TracTickets for help on using tickets.