golang defined php class not available after context destroy

[taowen]

package bug

import (
    "fmt"
    "github.com/deuill/go-php/engine"
    "os"
    "runtime"
    "testing"
)

type TestObj struct{}

func newTestObj(args []interface{}) interface{} {
    return &TestObj{}
}

func Test_bug(t *testing.T) {
    runtime.GOMAXPROCS(1)
    theEngine, err := engine.New()
    if err != nil {
        t.Fail()
    }
    theEngine.Define("TestObj", newTestObj)
    fmt.Println("1")
    context1, err := theEngine.NewContext()
    if err != nil {
        t.Fail()
    }
    fmt.Println("enter")
    _, err = context1.Eval("$testObj = new TestObj();")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    context1.Destroy()
    fmt.Println("1 done")
    fmt.Println("2")
    context2, err := theEngine.NewContext()
    context2.Output = os.Stdout
    if err != nil {
        t.Fail()
    }
    fmt.Println("enter")
    _, err = context2.Eval("$testObj = new TestObj();")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    context2.Destroy()
    fmt.Println("2 done")
    fmt.Println("all done")
}

The code will exit with 255. The php error log says

[27-Oct-2016 09:40:05 Asia/Chongqing] PHP Fatal error:  Class 'TestObj' not found in gophp-engine on line 1

However, if we change the code to

package bug

import (
    "fmt"
    "github.com/deuill/go-php/engine"
    "os"
    "runtime"
    "testing"
)

type TestObj struct{}

func newTestObj(args []interface{}) interface{} {
    return &TestObj{}
}

func Test_bug(t *testing.T) {
    runtime.GOMAXPROCS(1)
    theEngine, err := engine.New()
    if err != nil {
        t.Fail()
    }
    theEngine.Define("TestObj", newTestObj)
    fmt.Println("1")
    context1, err := theEngine.NewContext()
    if err != nil {
        t.Fail()
    }
    fmt.Println("enter")
    _, err = context1.Eval("$testObj = new TestObj();")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    context1.Destroy()
    fmt.Println("1 done")
    fmt.Println("2")
    context2, err := theEngine.NewContext()
    context2.Output = os.Stdout
    if err != nil {
        t.Fail()
    }
    fmt.Println("enter")
    _, err = context2.Eval("var_dump(new TestObj());")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    context2.Destroy()
    fmt.Println("2 done")
    fmt.Println("all done")
}

it finishes without any problem, and the output to console is:

1
enter
back
1 done
2
enter
object(TestObj)#1 (0) {
}
back
2 done
all done

Which means the TestObj class definition is not actually completely gone, after context1 destroyed.

[taowen]

package bug

import (
    "fmt"
    "github.com/deuill/go-php/engine"
    "os"
    "runtime"
    "testing"
)

type TestObj struct{}

func newTestObj(args []interface{}) interface{} {
    return &TestObj{}
}

func Test_bug(t *testing.T) {
    runtime.GOMAXPROCS(1)
    theEngine, err := engine.New()
    if err != nil {
        t.Fail()
    }
    fmt.Println("1")
    context1, err := theEngine.NewContext()
    if err != nil {
        t.Fail()
    }
    recv1, err := theEngine.Define("TestObj", newTestObj)
    if err != nil {
        t.Fail()
    }
    fmt.Println("enter")
    _, err = context1.Eval("$testObj = new TestObj();")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    recv1.Destroy()
    context1.Destroy()
    fmt.Println("1 done")
    fmt.Println("2")
    context2, err := theEngine.NewContext()
    if err != nil {
        t.Fail()
    }
    recv2, err := theEngine.Define("TestObj", newTestObj)
    if err != nil {
        t.Fail()
    }
    context2.Output = os.Stdout
    fmt.Println("enter")
    _, err = context2.Eval("$testObj = new TestObj();")
    fmt.Println("back")
    if err != nil {
        t.Fail()
    }
    recv2.Destroy()
    context2.Destroy()
    fmt.Println("2 done")
    fmt.Println("all done")
}

hack engine define to allow re-define the receiver again seems fixed the problem

func (e *Engine) Define(name string, fn func(args []interface{}) interface{}) (*Receiver, error) {
    //if _, exists := e.receivers[name]; exists {
    //  return fmt.Errorf("Failed to define duplicate receiver '%s'", name)
    //}

It seems like the php_request_shutdown(NULL); caused the global defined class unavailable.

[deuill]

Interesting, I'll check it out and see if I can make a quick fix for this.

[taowen]

can not reproduce the bug in php 7 with static linking, interesting

[deuill]

The semantics between destroying PHP requests (i.e. contexts) and modules (i.e. engines) are different between version 5.x and 7.x... I've had quite a few headaches trying to resolve issues where PHP would segfault for seemingly no apparent reason, when implementing the above code.

The reason I've attached the Define() method to the Engine rather than the Context receiver is because class definitions should survive a Context.Destroy() and be used by subsequent Context instances. It seems this isn't the case.

I'll add some test cases like the above and see if I can reproduce the issue.

[deuill]

I've been running variations of the above and cannot reproduce, on ArchLinux x64, PHP version 7.0.12. I'm gonna open a PR with tests covering the above and take it from there.

[borancar]

I was able to reproduce this locally both with PHP5 and PHP7 - the receiver is only available sometimes. With 7.1 and onwards, receiver_define will also segfault in register_interned_string. Ultimately this boils down to class registration not being in the context of a module (module is 0). If you look at any PHP/Zend extension, class registration will be done inside MINIT.

I fixed the receiver issue for myself by creating a fake module that handles registration inside MINIT. I reworked the receivers quite a bit for this - a list is provided to engine.New which then transfers the strings to the C side into a global array which is then read during MINIT (itself invoked by providing the module to php_module_startup).

I can publish my changes in case there's interest, but it will have to be a separate repo as I've spliced the code to have PHP 5 with ZTS but also containing changes from 0.11.

[deuill]

Any contribution would be welcome -- noted that PHP 5.x support is probably going to be phased out. Also noted that PHP 7.4 has a new FFI which may help integration, as the internals are moving targets, and as you may have surmised, not that stable/easy to integrate against.