search

paulsmith / gogeos

Go library for spatial data operations and geometric algorithms (Go bindings for GEOS)
http://paulsmith.github.io/gogeos/
MIT License
280 stars 79 forks source link

Segmentation violation (reproducible) #19

Closed nicerobot closed 4 years ago

nicerobot commented 7 years ago

Ignore this comment. Go to the update.

I'm sorry that this is going to be very vague but we haven't had the time to investigate too much.

The gist is, we have some gogeos WKT processing that pretty consistently crashes with a segmentation violation when compiled using 1.8.1. The same doesn't happen with 1.7.3.

The main diagnosis we tried was removing the concurrency, both with code changes and GOMAXPROCS=1 and various GOGC settings. It still seemed to consistently crash with 1.8.1.

We use a Dockerfile to compile the code to ensure that all the dependencies are stable between compilations.

One of the stacks:

fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x80 addr=0x0 pc=0x7f851a2f15f2]

runtime stack:
runtime.throw(0x97b351, 0x2a)
    /go/src/runtime/panic.go:596 +0x95
runtime.sigpanic()
    /go/src/runtime/signal_unix.go:274 +0x2db

goroutine 5789 [syscall, locked to thread]:
runtime.cgocall(0x85ca70, 0xc434b07d78, 0xc434b07da0)
    /go/src/runtime/cgocall.go:131 +0xe2 fp=0xc434b07d48 sp=0xc434b07d08
github.com/paulsmith/gogeos/geos._Cfunc_GEOSWKTReader_read_r(0x2adccf0, 0x7f850403d330, 0x7f850403d220, 0x0)
    github.com/paulsmith/gogeos/geos/_obj/_cgo_gotypes.go:2249 +0x4e fp=0xc434b07d78 sp=0xc434b07d48
github.com/paulsmith/gogeos/geos.cGEOSWKTReader_read.func1(0x2adccf0, 0x7f850403d330, 0x7f850403d220, 0xc429edf840)
    /src/github.com/paulsmith/gogeos/geos/cwrappers.go:871 +0xad fp=0xc434b07db0 sp=0xc434b07d78
github.com/paulsmith/gogeos/geos.cGEOSWKTReader_read(0x7f850403d330, 0x7f850403d220, 0x0)
    /src/github.com/paulsmith/gogeos/geos/cwrappers.go:871 +0x86 fp=0xc434b07de0 sp=0xc434b07db0
github.com/paulsmith/gogeos/geos.(*wktDecoder).decode(0xc430098b10, 0xc42a6e2bd0, 0x81, 0x0, 0x0, 0x0)
    /src/github.com/paulsmith/gogeos/geos/wkt.go:33 +0x9e fp=0xc434b07e10 sp=0xc434b07de0
github.com/paulsmith/gogeos/geos.FromWKT(0xc42a6e2bd0, 0x81, 0x81, 0x471f64, 0xc431037860)

The Dockerfile and build:

git clone https://gist.github.com/84ebf7c210c0b53ec971b7113b17a607.git
cd 84ebf7c210c0b53ec971b7113b17a607
make build
make run

(the main.go in that gist obviously doesn't represent our code which we can't post and haven't had time to create a reproducible case to post)

nicerobot commented 7 years ago

Here's a reproducible test case without the docker requirements. Just clone the gist and run make.

Seems to be lots of libraries/issues with a similar problem:

https://github.com/libgit2/git2go/issues/334 comment

I suspect this has the same root cause as #373 and #356 where Go's GC decides to free the managed object before we're done with it.

https://github.com/libgit2/git2go/issues/373 https://github.com/libgit2/git2go/pull/387 https://github.com/libgit2/git2go/issues/356 https://github.com/go-gitea/gitea/issues/1408 https://github.com/google/cadvisor/issues/1646 https://github.com/golang/go/issues/17384 https://github.com/golang/go/issues/18429 https://github.com/golang/go/issues/16331#issuecomment-313344965 https://github.com/paulsmith/gogeos/issues/11

nicerobot commented 7 years ago

@paulsmith from ianlancetaylor

At a quick glance the paulsmith/gogeos package looks risky, as it uses runtime.SetFinalizer but never uses runtime.KeepAlive. See https://golang.org/doc/go1.8#liveness .

furstenheim-geoblink commented 4 years ago

I think that the package is correct, but in a usage outside of the package one should be aware of runtime.KeepAlive