VoronoiDigramBuilder class + test #25

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@vishaltiwari

This should rebase now.

@vishaltiwari

I am having a memory leak at this line, not able to figure out why.

strk replied Sep 9, 2013

That's because GeometryFactory::createGeometryCollection, called on line 147 by passing the "clipped" vector by reference, makes a deep copy of all elements of the vector, so the ownership of those remains to the caller, which should take care of deleting them.
REF: https://github.com/libgeos/libgeos/blob/svn-trunk/include/geos/geom/GeometryFactory.h#L177

Rather than deleting all elements, I suggest you pass the whole ownershipt to createGeometryCollection, you can do that by heap-allocating the vector and passing it by-pointer.
REF: https://github.com/libgeos/libgeos/blob/svn-trunk/include/geos/geom/GeometryFactory.h#L173

I suggest you immediately put the vector into an auto_ptr online 125 and .release it when passed to GeometryFactory

@vishaltiwari

There is a problem that i am facing now, for the 6th test case its giving:

geos::triangulate::Voronoi: .....pure virtual method called
terminate called without an active exception
Aborted

I don't know which pure virtual method it's calling. Surprisingly it's not giving this error for the first 5 test cases. How do i deal with this?

@strk
Member
strk commented Sep 9, 2013

you run the test under gdb and get a backtrace.
I could reproduce:

Program received signal SIGABRT, Aborted.
0x00007ffff6c7b425 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff6c7b425 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007ffff6c7eb8b in __GI_abort () at abort.c:91
#2  0x00007ffff75cd69d in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff75cb846 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff75cb873 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff75cc28f in __cxa_pure_virtual () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff7af77f1 in geos::geom::GeometryCollection::normalize (this=0x9476c0) at GeometryCollection.cpp:247
#7  0x00000000005c274a in tut::runVoronoi (sitesWkt=<optimized out>, expectedWkt=<optimized out>, tolerance=0)
    at triangulate/VoronoiTest.cpp:62

Even better, you run trough valgrind and get more info:

==7968== Invalid read of size 8
==7968==    at 0x4EED7E8: geos::geom::GeometryCollection::normalize() (GeometryCollection.cpp:247)
==7968==    by 0x5C2749: tut::runVoronoi(char const*, char const*, double) (VoronoiTest.cpp:62)
==7968==    by 0x5C5697: tut::test_group<tut::test_voronoidiag_data, 50>::run_next(tut::test_result&) (tut.hpp:452)
==7968==    by 0x41D6BF: tut::test_runner::run_tests(std::string const&) const (tut_runner.hpp:323)
==7968==    by 0x411AC3: main (geos_unit.cpp:95)
==7968==  Address 0x6114088 is 24 bytes inside a block of size 64 free'd
==7968==    at 0x4C2A4BC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7968==    by 0x4EECD4E: geos::geom::GeometryCollection::~GeometryCollection() (GeometryCollection.cpp:359)
==7968==    by 0x4EECDB8: geos::geom::GeometryCollection::~GeometryCollection() (GeometryCollection.cpp:362)
==7968==    by 0x4F71D61: geos::triangulate::VoronoiDiagramBuilder::getDiagram(geos::geom::GeometryFactory const&) (auto_ptr.h:170)
==7968==    by 0x5C2729: tut::runVoronoi(char const*, char const*, double) (VoronoiTest.cpp:60)
==7968==    by 0x5C5697: tut::test_group<tut::test_voronoidiag_data, 50>::run_next(tut::test_result&) (tut.hpp:452)
==7968==    by 0x41D6BF: tut::test_runner::run_tests(std::string const&) const (tut_runner.hpp:323)
==7968==    by 0x411AC3: main (geos_unit.cpp:95)

Valgrind tells you that VoronoiDiagramBuilder::getDiagram removed the geometry collection too early
(lack of .release of an auto_ptr?)

@strk
Member
strk commented Sep 9, 2013

I see clipGeometryCollection is not copying an element of the input geometry which is fully contained in the envelope. Line 133. Instead "result" is owned when generated via ->intersection on line 137.

I think you should clone the one on line 133 instead. This is how JTS is different from GEOS... we sometimes have to copy data more as we can't count on a garbage collector (and we never switched to refcounting)

@strk
Member
strk commented Sep 9, 2013

besides: VoronoiDiagramBuilder.cpp:126:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

getNumGeometries returns std::size_t, use that for i as well

@vishaltiwari

cloned the geometry from GeometryCollection geometries and then storing the
returned pointer in result. Also changed int i to std::size_t i. Things are
working fine now. :) .Thanks

On 9 September 2013 22:36, strk notifications@github.com wrote:

besides: VoronoiDiagramBuilder.cpp:126:42: warning: comparison between
signed and unsigned integer expressions [-Wsign-compare]

getNumGeometries returns std::size_t, use that for i as well


Reply to this email directly or view it on GitHubhttps://github.com/libgeos/libgeos/pull/25#issuecomment-24095624
.

Vishal Tiwari
Undergraduate
Computer Science and Engineering
Lab for Spatial Informatics
IIIT-Hyderabad

@strk
Member
strk commented Sep 10, 2013

and you also included iostream :)

@vishaltiwari

removed iostream header :)

@strk strk closed this Sep 10, 2013
@mwtoews mwtoews pushed a commit to mwtoews/libgeos that referenced this pull request Mar 17, 2014
Sandro Santilli VoronoiDigramBuilder class + test
Contributed by Vishal Tiwari

See libgeos#25

git-svn-id: http://svn.osgeo.org/geos/trunk@3951 5242fede-7e19-0410-aef8-94bd7d2200fb
24c6fa8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment