GRASS driver crash after MAPSET unset

[rblazek]

The bug was first reported in qgis:  http://trac.osgeo.org/qgis/ticket/1900

The problem is that, if MAPSET environment variable is unset by an application, the next call of IRasterIO crashes because it calls G_set_window which needs a MAPSET to find if MASK exists. There are othere similar calls.

I have written new function ResetMapset which is called before any access to data. Patch is attached, should I commit to svn?

[rblazek]

patch

[rouault]

CC'ing Martin Landa as he might be interested

[rouault]

Radim, I'm not very qualified when it comes to GRASS, but I guess you can commit it if you think it's reasonnably safe to apply after your testing. The integration of GRASS in any application seem to be rather fragile due to the use of those global environment variables.

I see that ResetMapset?() could be called quite frequently as it is called from IRasterIO for example. Have you checked that it doesn't cause too many memory leaks ? I remember having tried to fix memory leaks in the driver in the past, but I just discovered that most of them were in the GRASS libs themselves.

[rblazek]

You are right, Gsetenv() calls set_env() which allocates memory for the new value using G_store() but does not release memory allocated for previous value. Unfortunately even G_unsetenv() does not release the memory and there is no other access to the store where variables are stored. I have created  http://trac.osgeo.org/grass/ticket/863

Looking into GRASS code, I also discovered the Gsetenv() is quite complex and it can consume a lot of time, it is bad to call it for each raster row (which is the case in qgis for example).

G_reset_mapsets() is fast but it does not release the memory.  http://trac.osgeo.org/grass/ticket/864

G_add_mapset_to_search_path() allocates memory with G_store() which is not released by G_reset_mapsets().

There is another possibility to fix the bug, just setting G_suppress_masking() disables checking for MASK file and the MAPSET is not used. It should also speed up the driver. I am convinced that MASK is not used much and maybe GDAL should ignore it in any case (I remember one advanced GRASS user becoming mad because of forgotten MASK set). OTOH, GRASS lib can change in future and require MAPSET to be set for other purpose and if GDAL begins to ignore MASK now, it means that GDAL behaviour changes.

[rblazek]

The patchx is bad as explained above. The solution would be to add to GRASS functions for switching between more environments as described here  http://trac.osgeo.org/grass/ticket/863#comment:4 Setting G_suppress_masking() would also possible and not harmful I think.

The original problem in QGIS was solved by writing native GRASS driver.