deuill / go-php

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

gc_possible_root: Assertion `(ref)->gc.u.v.type == 7 || (ref)->gc.u.v.type == 8' failed. #32

Closed taowen closed 7 years ago

taowen commented 7 years ago
package php

import (
    "testing"
    "github.com/stretchr/testify/assert"
    "github.com/deuill/go-php/engine"
)

func Test_map_value(t *testing.T) {
    should := assert.New(t)
    theEngine, err := engine.New()
    should.Nil(err)
    defer theEngine.Destroy()
    context, err := theEngine.NewContext()
    should.Nil(err)
    defer context.Destroy()
    context.Bind("hello", map[string]interface{}{})
}
    for _, v := range c.values {
        v.Destroy()
    }
    c.values = nil

    C.context_destroy(c.context)
    c.context = nil

when the values destroyed, then the context destroy will trigger gc_possible_root assertion error. Found in php-7.0.12

Complete error message

php.test: /home/xiaoju/disf-php/php-src/php-7.0.12/Zend/zend_gc.c:226: gc_possible_root: Assertion `(ref)->gc.u.v.type == 7 || (ref)->gc.u.v.type == 8' failed.
SIGABRT: abort
PC=0x7f051472e428 m=0
signal arrived during cgo execution
taowen commented 7 years ago

call zend_hash_str_del(&EG(symbol_table), name, strlen(name)); to remove bind name from symbol table seems like fixed the problem. Isn't the value destroy too brute force?

deuill commented 7 years ago

Possibly so, I'll need to delve into this a bit more since the semantics between what gets de-allocated when destroying values and contexts are still unclear to me... The current implementation does a zval_dtor, which seems to be the right thing to do, but maybe not.

taowen commented 7 years ago

I have found a fix for this problem. It is in my branch: https://github.com/taowen/go-php/blob/memory-leak-fix/engine/value.go

Problem 1, nested data structure memory leak

Array/Map works like a container owning its element. So when add zval to it, and it can keep track of its lifetime. However object property assignment is a copy of value, so after assign it, we need to free the zval created before, https://github.com/taowen/go-php/blob/memory-leak-fix/engine/value.go#L139

Problem 2, context.bind cause gc error

When a zval is added to symbol table. We no longer need to call zval_dtor. The zend vm will tear down the symbol table properly after shutdown. So, we just skip the destroying the value. Think context.bind as passing the memory ownership.

Problem 3, engine_value itself lifecycle

Because the engine_value itself is heap allocated. value_destroy will destroy the zval and engine_value wrapper heap memory. Sometime, we just want to destroy the wrapper, leaving the zval managed by the zend vm. It is too hard to keep track of two piece of memory (just like why zval instead zval* from php5 to php7). I just dropped the wrapper all together, using zval directly.

Now, I think all memory problem goes away. I am heading to create a php7 only version of this. It is just too much work to support both php5 and php7.