Skip to content

Continue running the go program after exit() in PHP #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thekid wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Continue running the go program after exit() in PHP #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d542f85
Continue running the go program after exit() in PHP
thekid Oct 1, 2018
d370fc7
Set error_reporting to E_ALL to ensure TestContextLog test works
thekid Oct 1, 2018
1896d38
Ensure triggering a fatal error will yield an ExitError
thekid Oct 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ engine_context *context_new() {
return context;
}

void context_exec(engine_context *context, char *filename) {
void context_exec(engine_context *context, char *filename, int *exit) {
int ret;

// Attempt to execute script file.
Expand All @@ -48,9 +48,11 @@ void context_exec(engine_context *context, char *filename) {
script.opened_path = NULL;
script.free_filename = 0;

ret = php_execute_script(&script);
ret = zend_execute_scripts(ZEND_REQUIRE, NULL, 1, &script);
Copy link
Owner

@deuill deuill Oct 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see php_execute_script does a bunch of other stuff but ends up calling zend_execute_scripts. Was this changed because php_execute_script will truncate EG(exit_status)?

*exit = -1;
} zend_catch {
errno = 1;
errno = 0;
*exit = EG(exit_status);
return;
} zend_end_try();

Expand All @@ -63,7 +65,7 @@ void context_exec(engine_context *context, char *filename) {
return;
}

void *context_eval(engine_context *context, char *script) {
void *context_eval(engine_context *context, char *script, int *exit) {
zval *str = _value_init();
_value_set_string(&str, script);

Expand All @@ -84,7 +86,13 @@ void *context_eval(engine_context *context, char *script) {

// Attempt to execute compiled string.
zval tmp;
_context_eval(op, &tmp);
_context_eval(op, &tmp, exit);

// Script called exit()
if (*exit != -1) {
Copy link
Owner

@deuill deuill Oct 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may be possible to skip changing the signature of the version-specific functions (_context_eval and _context_exec) and just use EG(exit_status) here, since nothing else should be using the same context concurrently.

errno = 0;
return NULL;
}

// Allocate result value and copy temporary execution result in.
zval *result = malloc(sizeof(zval));
Expand Down
24 changes: 22 additions & 2 deletions context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ type Context struct {
values []*Value
}

type ExitError struct {
Status int
}

func (e *ExitError) Error() string {
return fmt.Sprintf("Exitcode %d", e.Status)
}

// Bind allows for binding Go values into the current execution context under
// a certain name. Bind returns an error if attempting to bind an invalid value
// (check the documentation for NewValue for what is considered to be a "valid"
Expand All @@ -59,12 +67,18 @@ func (c *Context) Bind(name string, val interface{}) error {
func (c *Context) Exec(filename string) error {
f := C.CString(filename)
defer C.free(unsafe.Pointer(f))
var e C.int

_, err := C.context_exec(c.context, f)
_, err := C.context_exec(c.context, f, &e)
if err != nil {
return fmt.Errorf("Error executing script '%s' in context", filename)
}

code := int(e)
if code != -1 {
return &ExitError{code}
}

return nil
}

Expand All @@ -74,12 +88,18 @@ func (c *Context) Exec(filename string) error {
func (c *Context) Eval(script string) (*Value, error) {
s := C.CString(script)
defer C.free(unsafe.Pointer(s))
var e C.int

result, err := C.context_eval(c.context, s)
result, err := C.context_eval(c.context, s, &e)
if err != nil {
return nil, fmt.Errorf("Error executing script '%s' in context", script)
}

code := int(e)
if code != -1 {
return nil, &ExitError{code}
}

defer C.free(result)

val, err := NewValueFromPtr(result)
Expand Down
69 changes: 69 additions & 0 deletions context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,75 @@ func TestContextBind(t *testing.T) {
c.Destroy()
}

var exitTests = []struct {
code int
script string
}{
{
0,
"exit(0);",
},
{
1,
"exit(1);",
},
{
255,
"exit(255);",
},
{
255,
"trigger_error('test', E_USER_ERROR);",
},
}

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

for _, tt := range exitTests {
script, err := NewScript("exit", "<?php "+tt.script)
if err != nil {
t.Errorf("Could not create temporary file for testing")
continue
}

err = c.Exec(script.Name())
script.Remove()

exit, ok := err.(*ExitError)
if !ok {
t.Errorf("Expected an ExitError, have %v", err)
continue
}

if exit.Status != tt.code {
t.Errorf("Expected an exitcode of %d, have %d", tt.code, exit.Status)
}
}

c.Destroy()
}

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

for _, tt := range exitTests {
_, err := c.Eval(tt.script)

exit, ok := err.(*ExitError)
if !ok {
t.Errorf("Expected an ExitError, have %v", err)
continue
}

if exit.Status != tt.code {
t.Errorf("Expected an exitcode of %d, have %d", tt.code, exit.Status)
}
}

c.Destroy()
}

func TestContextDestroy(t *testing.T) {
c, _ := e.NewContext()
c.Destroy()
Expand Down
1 change: 1 addition & 0 deletions engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const char engine_ini_defaults[] = {
"expose_php = 0\n"
"default_mimetype =\n"
"html_errors = 0\n"
"error_reporting = E_ALL\n"
Copy link
Contributor Author

@thekid thekid Oct 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, this didn't have the desired effect of fixing the tests; unsure what is causing them to fail on Travis CI, then. However, these test failures have been around for a while looking at other pull requests.

Copy link
Owner

@deuill deuill Oct 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there have been some flaky tests that need fixing. I'll get on it.

"register_argc_argv = 1\n"
"implicit_flush = 1\n"
"output_buffering = 0\n"
Expand Down
4 changes: 2 additions & 2 deletions include/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ typedef struct _engine_context {
} engine_context;

engine_context *context_new();
void context_exec(engine_context *context, char *filename);
void *context_eval(engine_context *context, char *script);
void context_exec(engine_context *context, char *filename, int *exit);
void *context_eval(engine_context *context, char *script, int *exit);
void context_bind(engine_context *context, char *name, void *value);
void context_destroy(engine_context *context);

Expand Down
2 changes: 1 addition & 1 deletion include/php7/_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
#define ___CONTEXT_H___

static void _context_bind(char *name, zval *value);
static void _context_eval(zend_op_array *op, zval *ret);
static void _context_eval(zend_op_array *op, zval *ret, int *exit);

#endif
7 changes: 3 additions & 4 deletions src/php5/_context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ static void _context_bind(char *name, zval *value) {
ZEND_SET_SYMBOL(EG(active_symbol_table), name, value);
}

static void _context_eval(zend_op_array *op, zval *ret) {
static void _context_eval(zend_op_array *op, zval *ret, int *exit) {
zend_op_array *oparr = EG(active_op_array);
zval *retval = NULL;
zval **retvalptr = EG(return_value_ptr_ptr);
Expand All @@ -25,10 +25,9 @@ static void _context_eval(zend_op_array *op, zval *ret) {

zend_try {
Copy link
Owner

@deuill deuill Oct 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be zend_first_try too (my bad)?

zend_execute(op);
*exit = -1;
} zend_catch {
destroy_op_array(op);
efree(op);
zend_bailout();
*exit = EG(exit_status);
} zend_end_try();

destroy_op_array(op);
Expand Down
9 changes: 4 additions & 5 deletions src/php7/_context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@ static void _context_bind(char *name, zval *value) {
zend_hash_str_update(&EG(symbol_table), name, strlen(name), value);
}

static void _context_eval(zend_op_array *op, zval *ret) {
static void _context_eval(zend_op_array *op, zval *ret, int *exit) {
EG(no_extensions) = 1;

zend_try {
zend_first_try {
ZVAL_NULL(ret);
zend_execute(op, ret);
*exit = -1;
} zend_catch {
destroy_op_array(op);
efree_size(op, sizeof(zend_op_array));
zend_bailout();
*exit = EG(exit_status);
} zend_end_try();

destroy_op_array(op);
Expand Down