Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: panic if repeat defer mat.Close #899

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

opuqo
Copy link

@opuqo opuqo commented Sep 16, 2021

Simple code where problem was found:


import (
	"fmt"
	"gocv.io/x/gocv"
)

func main() {
	mat1 := gocv.NewMat()
	defer mat1.Close()
	mat2 := gocv.NewMat()
	mat1 = mat2
	defer mat2.Close()
}

If did't check on nil in func Close, get panic

[signal SIGSEGV: segmentation violation code=0x1 addr=0x7ff60000003c pc=0x4e3a79]

runtime stack:
runtime.throw(0x532266, 0x2a)
        /usr/local/go/src/runtime/panic.go:1116 +0x72
runtime.sigpanic()
        /usr/local/go/src/runtime/signal_unix.go:726 +0x4ac

goroutine 1 [syscall]:
runtime.cgocall(0x4d83a0, 0xc000050ea0, 0x0)
        /usr/local/go/src/runtime/cgocall.go:133 +0x5b fp=0xc000050e70 sp=0xc000050e38 pc=0x43b3bb
gocv.io/x/gocv._Cfunc_Mat_Close(0x7ff6f2380ce0)
        _cgo_gotypes.go:3080 +0x45 fp=0xc000050ea0 sp=0xc000050e70 pc=0x4d40c5```

@deadprogram deadprogram changed the base branch from release to dev September 19, 2021 19:29
@deadprogram
Copy link
Member

Hello @opuqo thanks for the contribution.

I am just wondering is returning an error really needed? Or perhaps just return might be sufficient to avoid the issue. Mainly we have wanted to make sure it gets called at all. The CI tests are failing in your branch because this does apparently happen someplace related to the profiling code.

Also, please note I have changed the branch to dev as mentioned here: https://github.com/hybridgroup/gocv/blob/release/CONTRIBUTING.md#how-to-use-our-github-repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants