GRASS GIS: Ticket #3635: Suspicious use and cleanup of the mapset tempoary directoryhttps://trac.osgeo.org/grass/ticket/3635
<p>
Similarly to wrong lockfile being removed in <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3631" title="#3631: defect: Mapset remains locked after switch to it and exit (closed: fixed)">#3631</a>, <code>grass.py</code> 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.
</p>
<p>
The clean up seems to be introduced in <a class="changeset" href="https://trac.osgeo.org/grass/changeset/54467" title="grass.py: clean up also location-based tmp dir
">r54467</a> (before 7.0). Note that <em>location</em> here means <em>full path to the mapset</em> and the variable is not changed after initialization to the "start time" mapset. At the same time <code>$GISBASE/etc/clean_temp</code> is being called. Why <code>clean_temp</code> is not sufficient?
</p>
<p>
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 <code>grass.py</code> indeed says "remove GUI session files from .tmp". Shouldn't it use the system temporary directory or always get the current mapset?
</p>
<p>
Consequently, after turning of GRASS GIS, the initial mapset's <code>.tmp</code> is removed completely while the current mapset's <code>.tmp</code> is cleaned in a sophisticated way using <code>clean_temp</code>. 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 <code>grass.py</code> should only do cleaning using <code>clean_temp</code>. Does this make sense or am I missing something?
</p>
en-usGRASS GIShttps://trac.osgeo.org/grass/chrome/site/grasslogo_vector_small.pnghttps://trac.osgeo.org/grass/ticket/3635
Trac 1.2.2wenzeslausTue, 04 Sep 2018 02:23:36 GMTowner changedhttps://trac.osgeo.org/grass/ticket/3635#comment:1
https://trac.osgeo.org/grass/ticket/3635#comment:1
<ul>
<li><strong>owner</strong>
changed from <em>grass-dev@…</em> to <em>wenzeslaus</em>
</li>
</ul>
<p>
The commit message for <a class="changeset" href="https://trac.osgeo.org/grass/changeset/54467" title="grass.py: clean up also location-based tmp dir
">r54467</a> does not specify why the change - addition of <code>os.path.join(location, ".tmp")</code> - was needed and what is its relation to <code>clean_temp</code>. (Note that "location" means full path to mapset; same below.)
</p>
<p>
Similar line is <code>cleanup_dir(os.path.join(location, ".tmp"))</code> 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 (<a class="changeset" href="https://trac.osgeo.org/grass/changeset/37863" title="Convert grass70 script to Python
">r37863</a>). It originated from <a class="changeset" href="https://trac.osgeo.org/grass/changeset/21048" title="work around MS-Windows TclTk event bug
">r21048</a> which was working around some bug related to Tcl/Tk GUI.
</p>
<p>
If GUI works as expected after <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3637" title="#3637: defect: Use the system temporary directory for all GUI rendering files (closed: fixed)">#3637</a>, the GUI files should disappear from the mapset tmp dir because all will be in the system tmp dir, then the line from <a class="changeset" href="https://trac.osgeo.org/grass/changeset/37863" title="Convert grass70 script to Python
">r37863</a> won't be needed if its comment refers to rendered files.
</p>
<p>
As for <a class="changeset" href="https://trac.osgeo.org/grass/changeset/54467" title="grass.py: clean up also location-based tmp dir
">r54467</a>, I don't see why <code>clean_temp</code> is not enough. So, unless I find something, e.g. that <code>clean_temp</code> is broken or somebody suggests why it is there, I'm going to remove this clean up and let <code>grass.py</code> rely on <code>clean_temp</code>.
</p>
<p>
<em>Preferred milestone: 7.8 (not in Trac yet)</em>
</p>
TicketnetelerTue, 04 Sep 2018 06:58:56 GMTmilestone changedhttps://trac.osgeo.org/grass/ticket/3635#comment:2
https://trac.osgeo.org/grass/ticket/3635#comment:2
<ul>
<li><strong>milestone</strong>
changed from <em>7.6.0</em> to <em>7.8.0</em>
</li>
</ul>
TicketwenzeslausSun, 16 Sep 2018 02:08:01 GMThttps://trac.osgeo.org/grass/ticket/3635#comment:3
https://trac.osgeo.org/grass/ticket/3635#comment:3
<p>
In <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73336" title="init: do not remove wrong mapset .tmp, rely on clean_temp (see #3635)
...">73336</a>:
</p>
<div class="message"><p>
init: do not remove wrong mapset .tmp, rely on clean_temp (see <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3635" title="#3635: defect: Suspicious use and cleanup of the mapset tempoary directory (closed: fixed)">#3635</a>)<br />
</p>
<p>
The previous behavior was deleting whole mapset temporary directory<br />
of a mapset set at the startup. When started in mapset A, switched to B,<br />
exited, whole .tmp of A was removed, B was cleaned by clean_temp.<br />
Now cleaning of A is done during switching (as before) but it is not<br />
touched again, so no running processed in a (new) session in A are<br />
in denger of loosing temporary files. The mapset active at exit is<br />
cleaning using clean_temp as before, but only by that.<br />
</p>
</div>
TicketwenzeslausSun, 16 Sep 2018 02:22:15 GMThttps://trac.osgeo.org/grass/ticket/3635#comment:4
https://trac.osgeo.org/grass/ticket/3635#comment:4
<p>
In <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73337" title="init: remove basically dead code to clean mapset .tmp in default ...">73337</a>:
</p>
<div class="message"><p>
init: remove basically dead code to clean mapset .tmp in default startup (see <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3635" title="#3635: defect: Suspicious use and cleanup of the mapset tempoary directory (closed: fixed)">#3635</a>)<br />
</p>
<p>
The default (fallback) startup contained code which was probably from the<br />
original Python rewrite (<a class="changeset" href="https://trac.osgeo.org/grass/changeset/37863" title="Convert grass70 script to Python
">r37863</a>) and maybe originated from workaround in <a class="changeset" href="https://trac.osgeo.org/grass/changeset/21048" title="work around MS-Windows TclTk event bug
">r21048</a>.<br />
The clean_temp tool is called and takes care of cleaning, so this should not<br />
be neccessary, not even for GUI (mentioned in the comment). GUI is not using<br />
mapset temporary directory since <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73334" title="wxGUI/core: replace grass.script.tempfile by ...">r73334</a> (<a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3637" title="#3637: defect: Use the system temporary directory for all GUI rendering files (closed: fixed)">#3637</a>).<br />
</p>
</div>
TicketwenzeslausSun, 16 Sep 2018 02:45:10 GMThttps://trac.osgeo.org/grass/ticket/3635#comment:5
https://trac.osgeo.org/grass/ticket/3635#comment:5
<p>
In <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73338" title="init: call clean_temp always at exit
This is has the same behaviour ...">73338</a>:
</p>
<div class="message"><p>
init: call clean_temp always at exit<br />
</p>
<p>
This is has the same behaviour as before, i.e. clean_temp called at all exits<br />
and it is similar to what was there before <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73336" title="init: do not remove wrong mapset .tmp, rely on clean_temp (see #3635)
...">r73336</a>, i.e. .tmp deleted at exit.<br />
Additionally this calls clean_temp also in some error states when then<br />
exist function calls in the call are not reached.<br />
</p>
<p>
See <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3635" title="#3635: defect: Suspicious use and cleanup of the mapset tempoary directory (closed: fixed)">#3635</a> for removal of Python-based clean up procedures (which should be covered by clean_temp).<br />
</p>
</div>
TicketwenzeslausSun, 16 Sep 2018 03:13:57 GMThttps://trac.osgeo.org/grass/ticket/3635#comment:6
https://trac.osgeo.org/grass/ticket/3635#comment:6
<p>
Now only the <code>clean_temp</code> tool is used. It is called through <code>atexit.register()</code>. <code>clean_temp</code> always cleans the current mapset (specified in an "rc file" pointed to by GISRC variable as for all GRASS modules).
</p>
<p>
What is now possible but was broken before:
</p>
<ol><li>Start session I in mapset A.
</li><li>Switch session I to mapset B.
</li><li>Start session II in mapset A.
</li><li>Start processes in session II (mapset A).
</li><li>Exit session I (from mapset B).
</li><li>Running processes in session II (mapset A) still have their temporary files.
</li></ol><p>
Before <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73336" title="init: do not remove wrong mapset .tmp, rely on clean_temp (see #3635)
...">r73336</a>, 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 <a class="changeset" href="https://trac.osgeo.org/grass/changeset/73334" title="wxGUI/core: replace grass.script.tempfile by ...">r73334</a> (<a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3637" title="#3637: defect: Use the system temporary directory for all GUI rendering files (closed: fixed)">#3637</a>).
</p>
<p>
By relying only on <code>clean_temp</code> and not deleting whole <code>.tmp</code> manually (leaving aside the mapset switching problem), there are two changes in behavior:
</p>
<ol><li>The <code>.tmp</code> directory and the <code>$HOSTNAME</code> subdirectory are never deleted once created. The content is cleaned by <code>clean_temp</code> which applies its rules rather than deleting all.
</li><li>When a process does not end in a standard way, e.g. by pressing <em>Stop</em> button in the wxGUI, the process may still show up when <code>clean_temp</code> 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 <code>__MINGW32__</code> 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.
</li></ol>
TicketwenzeslausSun, 16 Sep 2018 04:08:42 GMTstatus changed; resolution sethttps://trac.osgeo.org/grass/ticket/3635#comment:7
https://trac.osgeo.org/grass/ticket/3635#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Closing as fixed, but considering changes related to locking (<a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3631" title="#3631: defect: Mapset remains locked after switch to it and exit (closed: fixed)">#3631</a>), wxGUI rendering temporary files placement (<a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/3637" title="#3637: defect: Use the system temporary directory for all GUI rendering files (closed: fixed)">#3637</a>) and premature startup cleaning (<a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/613" title="#613: defect: clean_temp: not safe for concurrent use (closed: fixed)">#613</a>, <a class="closed ticket" href="https://trac.osgeo.org/grass/ticket/1286" title="#1286: defect: clean_temp can not be called before LOCATION_NAME is set (closed: fixed)">#1286</a>), please test and please open new tickets if you encounter issues. Here are some commands to help with testing:
</p>
<pre class="wiki">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;"
</pre><p>
</p>
Ticket