[GRASS-dev] [GRASS GIS] #863: set_env/unset_env should release memory

#863: set_env/unset_env should release memory
-------------------------+--------------------------------------------------
Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Keywords: | Platform: All
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
set_env/unset_env should release memory allocated for previously set value
if a name is reused/deleted.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863&gt;
GRASS GIS <http://grass.osgeo.org>

#863: set_env/unset_env should release memory
--------------------------+-------------------------------------------------
  Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords:
  Platform: All | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [ticket:863 rblazek]:
> set_env/unset_env should release memory allocated for previously set
value if a name is reused/deleted.

That would require G!__getenv() to duplicate the value. Currently, it just
returns the pointer. As the caller may hold onto the pointer indefinitely,
the library cannot free it. Duplicating the value would require the caller
to free it.

I'm not sure this is worth it. Do any GRASS modules call G_setenv()
repeatedly?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#863: set_env/unset_env should release memory
--------------------------+-------------------------------------------------
  Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords:
  Platform: All | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by rblazek):

GRASS modules probably don't set variables repeatedly, but in GDAL, for
example, if working with GRASS rasters from more mapsets/locations
simultaneously, needs to reset mapset/location if masking is not disabled.
It is using G_set_window() which checks for MASK.

Is there any other reasonable way how to reset environment when reading
rasters from more mapsets/locations?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#863: set_env/unset_env should release memory
--------------------------+-------------------------------------------------
  Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords:
  Platform: All | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:3 rblazek]:
> GRASS modules probably don't set variables repeatedly, but in GDAL, for
example, if working with GRASS rasters from more mapsets/locations
simultaneously, needs to reset mapset/location if masking is not disabled.
It is using G_set_window() which checks for MASK.
>
> Is there any other reasonable way how to reset environment when reading
rasters from more mapsets/locations?

If you need to switch between two distinct environments (e.g. source and
destination), there's G!__create_alt_env() and G!__switch_env().

If you need more than two environments, you're out of luck.

Changing this would mean that many library functions calling G_getenv()
etc would need to free the memory when it's no longer needed, which means
that they would need to either track when it's no longer needed or push
this burden onto the caller.

In large software packages, tracking dynamically-allocated memory can be a
major issue. Eliminating this issue is one of the main "selling points" of
high-level languages with built-in garbage collection.

The GRASS philosophy of not worrying about memory leaks which will only
occur a few times per process (for a normal GRASS module) saves us a
'''lot''' of trouble.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#863: set_env/unset_env should release memory
--------------------------+-------------------------------------------------
  Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords:
  Platform: All | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by rblazek):

Replying to [comment:4 glynn]:

Thanks for explanation. Two environments are not sufficient because
GDAL/QGIS can open any number of maps from different locations but the
idea of G!__switch_env() sounds good. Do you think that a patch adding
possibility to create more environments and switching between them using
environment id/index would be accepted? I am thinking about an array of
pointers to a structure keeping ENV *env and **mapset_name (maybe more?).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#863: set_env/unset_env should release memory
--------------------------+-------------------------------------------------
  Reporter: rblazek | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: normal | Milestone: 6.5.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords:
  Platform: All | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:5 rblazek]:

> Do you think that a patch adding possibility to create more environments
and switching between them using environment id/index would be accepted?

That seems reasonable.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/863#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>