[GRASS-dev] [GRASS GIS] #273: v.example leaks memory in db_open_select_cursor

#273: v.example leaks memory in db_open_select_cursor
---------------------+------------------------------------------------------
Reporter: karme | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: svn-trunk
Keywords: | Platform: Linux
      Cpu: x86-32 |
---------------------+------------------------------------------------------
example valgrind run:
{{{
==8090== 570,258 bytes in 569,794 blocks are definitely lost in loss
record 29 of 31
==8090== at 0x4023E8C: realloc (vg_replace_malloc.c:429)
==8090== by 0x4060670: db_realloc (alloc.c:72)
==8090== by 0x4064B74: db_enlarge_string (string.c:116)
==8090== by 0x406713A: db__recv_string (xdrstring.c:93)
==8090== by 0x4066294: db__recv_column_definition (xdrcolumn.c:29)
==8090== by 0x40675D8: db__recv_table_definition (xdrtable.c:35)
==8090== by 0x40FC4FB: db_open_select_cursor (c_openselect.c:55)
==8090== by 0x80499AC: main (main.c:165)
}}}

The problem is that each call to db_open_select_cursor => each database
query leaks memory.

It seems it doesn't matter which database backend you use.

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Changes (by karme):

  * component: default => Database

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by karme):

"proposed patch" is not quite true. I am not really sure - please review.
Thanks.

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by glynn):

Replying to [comment:2 karme]:
> "proposed patch" is not quite true. I am not really sure - please
review. Thanks.

The patch looks reasonable enough, although it's essentially impossible to
tell whether it will break anything. db_get_string() returns the pointer,
and if anything holds onto the pointer after db_free_column() returns, it
will break.

My inclination is to apply the patch. Unlike most GRASS memory "leaks",
this one could be significant. I.e. if it leaks memory for each row
fetched, it would impose a limit on the amount of data retrieved from a
query.

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by karme):

Replying to [comment:3 glynn]:
> My inclination is to apply the patch. Unlike most GRASS memory "leaks",
this one could be significant. I.e. if it leaks memory for each row
fetched, it would impose a limit on the amount of data retrieved from a
query.

Well it does not leak memory for each row fetched, but for each query. But
at the moment I am writing a v.out.osm that in the end does a query for
each line => you run out of memory quickly. It would be really nice to get
this fixed - maybe also in the stable branch? and maybe in debian/lenny?

A quick grep for db_open_select_cursor gives many hits and some of them
are in a inner loop. For example: v.out.ogr (in mk_att), d.vect (attr.c),
v.label, ...

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by glynn):

Replying to [comment:4 karme]:

> A quick grep for db_open_select_cursor gives many hits and some of them
are in a inner loop. For example: v.out.ogr (in mk_att), d.vect (attr.c),
v.label, ...

These modules should probably be making a single SELECT statement and
storing the results in a btree (or similar), rather than executing a
separate query for each line.

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by hamish):

Replying to [comment:4 karme]:
> maybe also in the stable branch?

if a fix makes it into trunk it is highly likely we will backport
it to the stable branch.

> and maybe in debian/lenny?

I doubt it. Lenny is now frozen and this is not a debian RC bug.
We are pretty much locked in to 6.2.3 there, but keep a close eye
out for a 6.3.0 (or 6.3.svn) version for Lenny from backports.org
soon after lenny is released. A 6.3.svn package will contain
critical backported fixes, but those are few. For the most part
normal bug fixes will first see daylight with the upcoming 6.4.

Hamish

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Comment (by karme):

I would suggest to apply this patch.

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

#273: v.example leaks memory in db_open_select_cursor
-----------------------+----------------------------------------------------
  Reporter: karme | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: closed
  Priority: major | Milestone: 6.4.0
Component: Database | Version: svn-trunk
Resolution: fixed | Keywords:
  Platform: Linux | Cpu: x86-32
-----------------------+----------------------------------------------------
Changes (by neteler):

  * status: new => closed
  * resolution: => fixed

Comment:

Fixed in GRASS 7 (r35066), 6.4.svn and 6.4.0svn (for RC2).

Markus

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