Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Panics on prepared geometry calls #11

Open
dim opened this issue Jan 10, 2015 · 2 comments
Open

Panics on prepared geometry calls #11

dim opened this issue Jan 10, 2015 · 2 comments

Comments

@dim
Copy link

dim commented Jan 10, 2015

I have observed panics when prepared geometry objects are used:

pure virtual method called
terminate called without an active exception
SIGABRT: abort
PC=0x7fbe473fbd27
signal arrived during cgo execution

goroutine 6 [syscall, locked to thread]:
runtime.cgocall_errno(0x408920, 0xc2080d9218, 0x0)
  $GOROOT/src/runtime/cgocall.go:130 +0xf5 fp=0xc2080d91f8 sp=0xc2080d91d0
github.com/paulsmith/gogeos/geos._Cfunc_GEOSPreparedCovers_r(0x2a7eb50, 0x7fbe3c127880, 0x2bb87e0, 0xc208034900)
  github.com/paulsmith/gogeos/geos/_obj/_cgo_gotypes.go:672 +0x40 fp=0xc2080d9218 sp=0xc2080d91f8
github.com/paulsmith/gogeos/geos.cGEOSPreparedCovers(0x7fbe3c127880, 0x2bb87e0, 0x4d3200)
  $GOPATH/src/github.com/paulsmith/gogeos/geos/cwrappers.go:547 +0x74 fp=0xc2080d9240 sp=0xc2080d9218
github.com/paulsmith/gogeos/geos.(*PGeometry).predicate(0xc20802e068, 0x7c8480, 0x6, 0x87a250, 0xc20802e600, 0x8, 0x0, 0x0)
  $GOPATH/src/github.com/paulsmith/gogeos/geos/prepared.go:95 +0x5b fp=0xc2080d92b0 sp=0xc2080d9240
github.com/paulsmith/gogeos/geos.(*PGeometry).Covers(0xc20802e068, 0xc20802e600, 0x348, 0x0, 0x0)
  $GOPATH/src/github.com/paulsmith/gogeos/geos/prepared.go:55 +0x63 fp=0xc2080d92f8 sp=0xc2080d92b0

I tried to recreate the behaviour in a test, but I couldn't, at least not consistently. It is related to Go's GC cycles and the finalizer calling cGEOSGeom_destroy(ptr) on the original Geometry. As a workaround, I added:

diff --git a/geos/prepared.go b/geos/prepared.go
index a7af6d1..3383fe8 100644
--- a/geos/prepared.go
+++ b/geos/prepared.go
@@ -14,12 +14,13 @@ import (
 // optimized for a limited set of operations.
 type PGeometry struct {
        p *C.GEOSPreparedGeometry
+       g *Geometry
 }

 // PrepareGeometry constructs a prepared geometry from a normal geometry object.
 func PrepareGeometry(g *Geometry) *PGeometry {
        ptr := cGEOSPrepare(g.g)
-       p := &PGeometry{ptr}
+       p := &PGeometry{ptr, g}
        runtime.SetFinalizer(p, (*PGeometry).destroy)
        return p
 }
@@ -27,6 +28,7 @@ func PrepareGeometry(g *Geometry) *PGeometry {
 func (p *PGeometry) destroy() {
        cGEOSPreparedGeom_destroy(p.p)
        p.p = nil
+       p.g = nil
 }

 // Prepared geometry binary predicates

It fixes the problem by preventing the Geometry from being garbage-collected, but it is not very elegant.

Thoughts? Thanks

@tbonfort
Copy link

tbonfort commented Jan 8, 2017

can confirm issue and fix

fatal error: unexpected signal during runtime execution
[signal 0xb code=0x1 addr=0xaa pc=0x7f378c89156f]

runtime stack:
runtime.throw(0x55b049308440, 0x2a)
	/usr/lib/go-1.6/src/runtime/panic.go:547 +0x90
runtime.sigpanic()
	/usr/lib/go-1.6/src/runtime/sigpanic_unix.go:12 +0x5a

goroutine 1 [syscall, locked to thread]:
runtime.cgocall(0x55b048ffd4c0, 0xc820118290, 0x10000c800000000)
	/usr/lib/go-1.6/src/runtime/cgocall.go:123 +0x11b fp=0xc820118258 sp=0xc820118228
github.com/paulsmith/gogeos/geos._Cfunc_GEOSPreparedContains_r(0x55b0498fc900, 0x7f3784001410, 0x7f3784001e58, 0x0)
	github.com/paulsmith/gogeos/geos/_obj/_cgo_gotypes.go:1427 +0x3e fp=0xc820118290 sp=0xc820118258
github.com/paulsmith/gogeos/geos.cGEOSPreparedContains(0x7f3784001410, 0x7f3784001e58, 0x3fe0dfffffffff00)
	/home/tbonfort/dev/go/src/github.com/paulsmith/gogeos/geos/cwrappers.go:529 +0x188 fp=0xc820118300 sp=0xc820118290
github.com/paulsmith/gogeos/geos.(*PGeometry).predicate(0xc82002a378, 0x55b049245e50, 0x8, 0x55b04935f6c8, 0xc820120850, 0x404588d150889301, 0x0, 0x0)
	/home/tbonfort/dev/go/src/github.com/paulsmith/gogeos/geos/prepared.go:95 +0x52 fp=0xc820118378 sp=0xc820118300
github.com/paulsmith/gogeos/geos.(*PGeometry).Contains(0xc82002a378, 0xc820120850, 0x1, 0x0, 0x0)
	/home/tbonfort/dev/go/src/github.com/paulsmith/gogeos/geos/prepared.go:37 +0x58 fp=0xc8201183c0 sp=0xc820118378

pmezard added a commit to pmezard/gogeos that referenced this issue Jan 10, 2017
The problem is twofold:
- In Geometry.Coords() we have to make sure the parent Geometry outlives
  the processing of the coordinates array.
- The previous condition is not enough when the geometry comes from
  Shell() or Holes(), which return internal objects. We have to make
  sure the shell/hole parent geometry outlives the shell/hole instance.

The fix is similar to the one suggested in paulsmith#11. There are probably a lot
of places where either previous conditions are required, I have not
checked all the code.

Note: this change the minimum required Go version to 1.7. I tried to
play build tag games to have empty versions of the runtime.KeepAlive
call, and failed.

Fixes paulsmith#8
@josebalius
Copy link

I seem to be having the same problem when creating geometries from WKT. Anyone have any ideas on a general solution?

jdejesus007 added a commit to Jlectronix/gogeos that referenced this issue Nov 26, 2019
The problem is twofold:
- In Geometry.Coords() we have to make sure the parent Geometry outlives
  the processing of the coordinates array.
- The previous condition is not enough when the geometry comes from
  Shell() or Holes(), which return internal objects. We have to make
  sure the shell/hole parent geometry outlives the shell/hole instance.

The fix is similar to the one suggested in paulsmith#11. There are probably a lot
of places where either previous conditions are required, I have not
checked all the code.

Note: this change the minimum required Go version to 1.7. I tried to
play build tag games to have empty versions of the runtime.KeepAlive
call, and failed.

Fixes paulsmith#8

per -> pmezard
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants