Skip to content

Commit

Permalink
Remove NewReceiver method, move logic to Context method receiver
Browse files Browse the repository at this point in the history 
This removes the `NewReceiver` method and moves logic for the `Define`
method from the `Engine` method receiver to `Context`.

Moving the `Define` method was required due to a bug/feature where
destroying a `Context` before all defined Receivers have also been
destroyed causes memory errors. Tying method receiver definitions
to the `Context` lifecycle implicitly fixes this issue.

This approach, however, contains a possible flaw, since class names
in PHP are defined in the global scope (i.e. the `Engine`), and
destroying a defined `Receiver` also removes the class entry for that
name, supposedly across all running contexts.
  • Loading branch information
@deuill
deuill committed Feb 19, 2016
1 parent 7add2a7 commit c1905f3
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 100 deletions.
40 changes: 38 additions & 2 deletions engine/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package engine
//
// #include <stdlib.h>
// #include <main/php.h>
// #include "receiver.h"
// #include "context.h"
import "C"

Expand All @@ -30,8 +31,9 @@ type Context struct {
// Header represents the HTTP headers set by current PHP context.
Header http.Header

context *C.struct__engine_context
values []*Value
context *C.struct__engine_context
values []*Value
receivers map[string]*Receiver
}

// Bind allows for binding Go values into the current execution context under
Expand All @@ -53,6 +55,34 @@ func (c *Context) Bind(name string, val interface{}) error {
return nil
}

// Define registers a PHP class for the name passed, using function fn as
// constructor for individual object instances as needed by the PHP context.
//
// The class name registered is assumed to be unique for the active engine.
//
// The constructor function accepts a slice of arguments, as passed by the PHP
// context, and should return a method receiver instance, or nil on error (in
// which case, an exception is thrown on the PHP object constructor).
func (c *Context) Define(name string, fn func(args []interface{}) interface{}) error {
if _, exists := c.receivers[name]; exists {
return fmt.Errorf("Failed to define duplicate receiver '%s'", name)
}

rcvr := &Receiver{
name: name,
create: fn,
objects: make([]*ReceiverObject, 0),
}

n := C.CString(name)
defer C.free(unsafe.Pointer(n))

C.receiver_define(n, unsafe.Pointer(rcvr))
c.receivers[name] = rcvr

return nil
}

// Exec executes a PHP script pointed to by filename in the current execution
// context, and returns an error, if any. Output produced by the script is
// written to the context's pre-defined io.Writer instance.
Expand Down Expand Up @@ -99,6 +129,12 @@ func (c *Context) Destroy() {
return
}

for _, r := range c.receivers {
r.Destroy()
}

c.receivers = nil

for _, v := range c.values {
v.Destroy()
}
Expand Down
22 changes: 22 additions & 0 deletions engine/context_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ func TestContextNew(t *testing.T) {
c.Destroy()
}

func TestContextDefine(t *testing.T) {
ctor := func(args []interface{}) interface{} {
return nil
}

c, _ := e.NewContext()

if err := c.Define("TestDefine", ctor); err != nil {
t.Errorf("Context.Define(): %s", err)
}

if len(c.receivers) != 1 {
t.Errorf("Context.Define(): `Context.receivers` length is %d, should be 1", len(c.receivers))
}

if err := c.Define("TestDefine", ctor); err == nil {
t.Errorf("Context.Define(): Incorrectly defined duplicate receiver")
}

c.Destroy()
}

var execTests = []struct {
name string
script string
Expand Down
38 changes: 7 additions & 31 deletions engine/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ import (

// Engine represents the core PHP engine bindings.
type Engine struct {
engine *C.struct__php_engine
contexts []*Context
receivers map[string]*Receiver
engine *C.struct__php_engine
contexts []*Context
}

// New initializes a PHP engine instance on which contexts can be executed. It
Expand All @@ -39,9 +38,8 @@ func New() (*Engine, error) {
}

e := &Engine{
engine: ptr,
contexts: make([]*Context, 0),
receivers: make(map[string]*Receiver),
engine: ptr,
contexts: make([]*Context, 0),
}

return e, nil
Expand All @@ -52,8 +50,9 @@ func New() (*Engine, error) {
// corresponds to PHP's RINIT (request init) phase.
func (e *Engine) NewContext() (*Context, error) {
ctx := &Context{
Header: make(http.Header),
values: make([]*Value, 0),
Header: make(http.Header),
values: make([]*Value, 0),
receivers: make(map[string]*Receiver),
}

ptr, err := C.context_new(unsafe.Pointer(ctx))
Expand All @@ -67,35 +66,12 @@ func (e *Engine) NewContext() (*Context, error) {
return ctx, nil
}

// Define registers a PHP class under the name passed, using fn as the class
// constructor.
func (e *Engine) Define(name string, fn func(args []interface{}) interface{}) error {
if _, exists := e.receivers[name]; exists {
return fmt.Errorf("Failed to define duplicate receiver '%s'", name)
}

rcvr, err := NewReceiver(name, fn)
if err != nil {
return err
}

e.receivers[name] = rcvr

return nil
}

// Destroy shuts down and frees any resources related to the PHP engine bindings.
func (e *Engine) Destroy() {
if e.engine == nil {
return
}

for _, r := range e.receivers {
r.Destroy()
}

e.receivers = nil

for _, c := range e.contexts {
c.Destroy()
}
Expand Down
25 changes: 2 additions & 23 deletions engine/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestEngineNew(t *testing.T) {
t.Fatalf("New(): %s", err)
}

if e.engine == nil || e.contexts == nil || e.receivers == nil {
if e.engine == nil || e.contexts == nil {
t.Fatalf("New(): Struct fields are `nil` but no error returned")
}
}
Expand All @@ -60,31 +60,10 @@ func TestEngineNewContext(t *testing.T) {
}
}

func TestEngineDefine(t *testing.T) {
ctor := func(args []interface{}) interface{} {
return nil
}

err := e.Define("TestDefine", ctor)
if err != nil {
t.Errorf("Define(): %s", err)
}

if len(e.receivers) != 1 {
t.Errorf("Define(): `Engine.receivers` length is %d, should be 1", len(e.receivers))
}

err = e.Define("TestDefine", ctor)
if err == nil {
t.Errorf("Define(): Incorrectly defined duplicate receiver")
}

}

func TestEngineDestroy(t *testing.T) {
e.Destroy()

if e.engine != nil || e.contexts != nil || e.receivers != nil {
if e.engine != nil || e.contexts != nil {
t.Errorf("Engine.Destroy(): Did not set internal fields to `nil`")
}

Expand Down
38 changes: 8 additions & 30 deletions engine/receiver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import (
"unsafe"
)

type object struct {
// ReceiverObject represents an object instance of a pre-defined method receiver.
type ReceiverObject struct {
instance interface{}
values map[string]reflect.Value
methods map[string]reflect.Value
Expand All @@ -27,30 +28,7 @@ type object struct {
type Receiver struct {
name string
create func(args []interface{}) interface{}
objects []*object
}

// NewReceiver registers a PHP class for the name passed, using function fn as
// constructor for individual object instances as needed by the PHP context.
//
// The class name registered is assumed to be unique for the active engine.
//
// The constructor function accepts a slice of arguments, as passed by the PHP
// context, and should return a method receiver instance, or nil on error (in
// which case, an exception is thrown on the PHP object constructor).
func NewReceiver(name string, fn func(args []interface{}) interface{}) (*Receiver, error) {
rcvr := &Receiver{
name: name,
create: fn,
objects: make([]*object, 0),
}

n := C.CString(name)
defer C.free(unsafe.Pointer(n))

C.receiver_define(n, unsafe.Pointer(rcvr))

return rcvr, nil
objects []*ReceiverObject
}

// Destroy removes references to the generated PHP class for this receiver and
Expand Down Expand Up @@ -80,7 +58,7 @@ func receiverNew(rcvr unsafe.Pointer, args unsafe.Pointer) unsafe.Pointer {

defer va.Destroy()

obj := &object{
obj := &ReceiverObject{
instance: r.create(va.Slice()),
values: make(map[string]reflect.Value),
methods: make(map[string]reflect.Value),
Expand Down Expand Up @@ -120,7 +98,7 @@ func receiverNew(rcvr unsafe.Pointer, args unsafe.Pointer) unsafe.Pointer {

//export receiverGet
func receiverGet(obj unsafe.Pointer, name *C.char) unsafe.Pointer {
o := (*object)(obj)
o := (*ReceiverObject)(obj)
n := C.GoString(name)

if _, exists := o.values[n]; !exists || !o.values[n].CanInterface() {
Expand All @@ -137,7 +115,7 @@ func receiverGet(obj unsafe.Pointer, name *C.char) unsafe.Pointer {

//export receiverSet
func receiverSet(obj unsafe.Pointer, name *C.char, val unsafe.Pointer) {
o := (*object)(obj)
o := (*ReceiverObject)(obj)
n := C.GoString(name)

// Do not attempt to set non-existing or unset-able field.
Expand All @@ -155,7 +133,7 @@ func receiverSet(obj unsafe.Pointer, name *C.char, val unsafe.Pointer) {

//export receiverExists
func receiverExists(obj unsafe.Pointer, name *C.char) C.int {
o := (*object)(obj)
o := (*ReceiverObject)(obj)
n := C.GoString(name)

if _, exists := o.values[n]; !exists {
Expand All @@ -167,7 +145,7 @@ func receiverExists(obj unsafe.Pointer, name *C.char) C.int {

//export receiverCall
func receiverCall(obj unsafe.Pointer, name *C.char, args unsafe.Pointer) unsafe.Pointer {
o := (*object)(obj)
o := (*ReceiverObject)(obj)
n := C.GoString(name)

if _, exists := o.methods[n]; !exists {
Expand Down
31 changes: 17 additions & 14 deletions engine/receiver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func newTestReceiver(args []interface{}) interface{} {
return &testReceiver{Var: value, hidden: 42}
}

var newReceiverTests = []struct {
var receiverDefineTests = []struct {
script string
expected string
}{
Expand All @@ -65,7 +65,6 @@ var newReceiverTests = []struct {
}`,
"Failed to instantiate method receiver",
},

{
"$t = new TestReceiver; echo $t->Var;",
"Foo",
Expand All @@ -78,7 +77,6 @@ var newReceiverTests = []struct {
"$t = new TestReceiver('wow'); echo $t->Var;",
"wow",
},

{
"$t = new TestReceiver; $t->Var = 'Bar'; echo $t->Var;",
"Bar",
Expand All @@ -87,7 +85,6 @@ var newReceiverTests = []struct {
"$t = new TestReceiver; $t->hello = 'wow'; echo $t->hello;",
"",
},

{
"$t = new TestReceiver; echo $t->Ignore();",
"",
Expand All @@ -104,7 +101,6 @@ var newReceiverTests = []struct {
"$t = new TestReceiver; echo $t->invalid();",
"",
},

{
"$t = new TestReceiver; echo ($t->Var) ? 1 : 0;",
"1",
Expand All @@ -123,18 +119,22 @@ var newReceiverTests = []struct {
},
}

func TestNewReceiver(t *testing.T) {
func TestReceiverDefine(t *testing.T) {
var w bytes.Buffer

c, _ := e.NewContext()
c.Output = &w

r, err := NewReceiver("TestReceiver", newTestReceiver)
if err != nil {
t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
if err := c.Define("TestReceiver", newTestReceiver); err != nil {
t.Fatalf("Engine.Define(): Failed to define method receiver: %s", err)
}

for _, tt := range newReceiverTests {
// Attempting to define a receiver twice should fail.
if err := c.Define("TestReceiver", newTestReceiver); err == nil {
t.Fatalf("Engine.Define(): Defining duplicate receiver should fail")
}

for _, tt := range receiverDefineTests {
_, err := c.Eval(tt.script)
if err != nil {
t.Errorf("Context.Eval('%s'): %s", tt.script, err)
Expand All @@ -149,17 +149,20 @@ func TestNewReceiver(t *testing.T) {
}
}

r.Destroy()
c.Destroy()
}

func TestReceiverDestroy(t *testing.T) {
c, _ := e.NewContext()
defer c.Destroy()

r, err := NewReceiver("TestReceiver", newTestReceiver)
if err != nil {
t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
if err := c.Define("TestReceiver", newTestReceiver); err != nil {
t.Fatalf("Engine.Define(): Failed to define method receiver: %s", err)
}

r := c.receivers["TestReceiver"]
if r == nil {
t.Fatalf("Receiver.Destroy(): Could not find defined receiver")
}

r.Destroy()
Expand Down

0 comments on commit c1905f3

Please sign in to comment.