Double free error when cloning repository about git2go HOT 32 CLOSED

It deallocates it on error, because it doesn't return it. If there's an error, it's something else.

from git2go.

I see, What is interesting, though, is that my double free error went away when I commented the finalizer. I will keep digging more anyway.

from git2go.

It also goes away if I change this:

func (v *Repository) Free() {
    runtime.SetFinalizer(v, nil)
    C.git_repository_free(v.ptr)
}

to

func (v *Repository) Free() {
    runtime.SetFinalizer(v, nil)
    if v.ptr != nil {
        C.git_repository_free(v.ptr)
    }
}

I agree the real culprit is still unknown. I'm going to check next if the finalizer is being called more than once by Go.

from git2go.

After some more testing, it is now looking more like a race condition somewhere inside git2go. This is what I'm doing, I'm starting multiple goroutines at once, each one cloning a different repository. I wait for all of them to finish and return back the result. It sometimes finishes fine and sometimes it is not.

from git2go.

Or maybe I'm misusing git2go? This is what I'm doing:

log.Info("Downloading buildpacks...")
    for _, bp := range bpacks {
        wg.Add(1)
        go func(url string) {
            log.Info("Cloning " + url + " into " + bpacksPath)

            defer wg.Done()
            parts := strings.Split(url, "#")

            branch := "master"
            if len(parts) == 2 {
                branch = parts[1]
            }

            dest := filepath.Join(bpacksPath, path.Base(url))

            options := &git.CloneOptions{
                CheckoutBranch: branch,
            }
            _, err := git.Clone(url, dest, options)
            if err != nil {
                log.Error(err.Error())
            }
        }(bp)
    }

    wg.Wait()

from git2go.

That definitely looks like it should work.

The check for v.ptr != nil suggests that we're returning a repository object when there was an error creating it.

from git2go.

I also tested checking for v.ptr != nil in Repository.Free() but I still get the double free errors coming from inside git_clone()

from git2go.

That trace you posted says it goes through clone.go:48, but in master that's the defer call which wouldn't be causing this? Are you using something other than master?

from git2go.

Sorry, I should have mentioned that before, I'm using your make-static branch.

from git2go.

Ok, that has the same clone code. I can't reproduce locally with code that should behave the same, but with more fixed data.

I had added a line to the imports, so of course line 48 is the actual git_clone() call which suggests it's an issue in libgit2 which is causing the double-free and the finalizer shouldn't have anything to do with it...

package main

import (
    "io/ioutil"
    "sync"
    "log"
    "github.com/libgit2/git2go"
    "runtime"
)

func main() {
    list := []string {
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
        "git://localhost/git/git-httpd",
    }

    var wg sync.WaitGroup

    for _, repo := range list {
        wg.Add(1)
        go func(url string) {
            defer wg.Done()

            path, err := ioutil.TempDir("", "clone-test-clone")
            if err != nil {
                log.Fatal(":(", err)
            }

            opts := &git.CloneOptions{
                CheckoutBranch: "master",
            }
            _, err = git.Clone(url, path, opts)
            if err != nil {
                log.Println(" -- ", err)
            }

        }(repo)
    }

    wg.Wait()
    runtime.GC()
}

from git2go.

These are the URLs I'm using:

var buildpacks = []string{
    //Heroku official buildpacks
    "https://github.com/heroku/heroku-buildpack-play.git",
    "https://github.com/heroku/heroku-buildpack-scala.git",
    "https://github.com/heroku/heroku-buildpack-java.git",
    "https://github.com/heroku/heroku-buildpack-ruby.git",
    "https://github.com/heroku/heroku-buildpack-nodejs.git",
    "https://github.com/heroku/heroku-buildpack-python.git",
    "https://github.com/heroku/heroku-buildpack-clojure.git",
    "https://github.com/heroku/heroku-buildpack-gradle.git",
    "https://github.com/heroku/heroku-buildpack-grails.git",
    "https://github.com/heroku/heroku-buildpack-php.git",

    //Community buildpacks
    "https://github.com/ddollar/heroku-buildpack-multi.git",
    "https://github.com/kr/heroku-buildpack-go.git",
    "https://github.com/jruby/heroku-buildpack-jruby.git",
    "https://github.com/archaelus/heroku-buildpack-erlang.git",
    "https://github.com/atris/heroku-buildpack-C.git",
    "https://github.com/miyagawa/heroku-buildpack-perl.git",
    "https://github.com/igrigorik/heroku-buildpack-dart.git",
    "https://github.com/ericfode/heroku-buildpack-rust.git",
    "https://github.com/oortcloud/heroku-buildpack-meteorite.git",
}

from git2go.

As you mentioned, at this point I think it is clear the finalizer is not the culprit.

from git2go.

It looks like we don't really handle using SSL from multiple threads too well, so that explains why using git:// didn't fail.

from git2go.

This should be fixed with libgit2/libgit2#2421, could you try it out?

from git2go.

I checked out your branch with the fix into the vendor folder, and re-installed libgit2. But, I still get the same errors: https://gist.github.com/c4milo/cd4d67ae5985a83ebc5d

from git2go.

Make sure you remove the old version from $GOPATH/pkg (e.g. rm $GOPATH/pkg/github.com/libgit2/git2go.a ) because the build tool won't notice when only libgit2 has changed and will no-op if no go files have changed.

from git2go.

If that doesn't fix it, could you attach gdb and show a backtrace?

from git2go.

Removed git2go.a and I still get the errors. Regarding attaching GDB, I'm using OSX Mavericks and currently running into this issue: http://stackoverflow.com/questions/19745549/golang-cannot-get-gdb-working-for-go-programs-using-c-libraries

from git2go.

lldb should work just as well. I'll go test with my OSX machine in a bit.

from git2go.

Oh nice. Yeah, I was able to use lldb just fine. Here is the backtrace https://gist.github.com/c4milo/357416303ae4ad9087e1

from git2go.

Turns out our handling of openssl with threading is completely broken, I'm working on fixing it properly.

from git2go.

I'm glad you found the problem. 👯 Looking forward to your patch.

from git2go.

Alright, now it's fixed for real, can you test with the current version of cmn/ssl-init-once? It now passes for me on OSX too. I wonder if it's there's a difference in the scheduler, since all these calls down to C land make it use real threads for the goroutines.

from git2go.

It seems to happen less often but I'm still getting errors.

from git2go.

This is a new stack trace:

from git2go.

OpenSSL seems to be very picky about where it gets initialised and isn't explicit about it. In the branch I've moved to init it during the general library init, which is not great, but seems to be required. Could you take another look?

I used to be able to reproduce with the last few commits by setting a high GOMAXPROCS which seemed to make go happier to start up threads. With the 'wip' commit, that doesn't do it anymore.

from git2go.

@carlosmn I'm not sure I understand what you are asking me to do.

from git2go.

ah sorry, I get it now. I'm going to test again and let you know my findings.

from git2go.

So far, I haven't been able to reproduce it again. I think that did it @carlosmn. It seems to be a bit slower than before but it could also be due to other factors. Nice work.

from git2go.

There is a printf on each lock and unlock which is going to slow it down quite a bit, but once the real commit is in it should barely affect the speed.

from git2go.

Makes sense. Thank you for explaining :)

from git2go.

libgit2 was fixed a while ago, and we're using a fixed version in vendor/libgit2. Closing as fixed.

from git2go.