deuill / go-php

PHP bindings for the Go programming language (Golang)
MIT License
930 stars 105 forks source link

fix memory leak #34

Closed taowen closed 7 years ago

taowen commented 7 years ago

build php7 in debug mode will show the leaks

taowen commented 7 years ago

the fix is flawed

taowen commented 7 years ago

ADD_REF seems fixed the case: when array embedded in array

taowen commented 7 years ago

I think I know the problem

func TestCopy(t *testing.T) {
    e, _ = New()
    defer e.Destroy()
    c, _ := e.NewContext()
    defer c.Destroy()
    c.Bind("b", []interface{}{
        []string{"hello"},
    })
    c.Eval("$a = $b;")
}

It will show the gc_possible_root error. The refcounted part of zval is held by the $a, so that zend will try to gc it. but we have destroyed the whole value from $b. The correct solution is to NOT zval_dtor, instead we let the reference counting do the job, which is how zval in php7 is designed to behave.

deuill commented 7 years ago

Thanks for opening this, the investigation is valuable regardless of whether this goes in as-is or not. I'm aware of edge-cases where memory isn't freed correctly on shutdown, and will continue to investigate and get to the bottom of this.