-
Notifications
You must be signed in to change notification settings - Fork 105
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.
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
Unexpected signal during runtime execution #62
Comments
Everything works fine if I remove the TestReceiverDestroy test and move the cleanup code before the $ git diff
diff --git a/receiver_test.go b/receiver_test.go
index 8c662b0..fa3130c 100644
--- a/receiver_test.go
+++ b/receiver_test.go
@@ -149,13 +149,6 @@ func TestReceiverDefine(t *testing.T) {
}
}
- c.Destroy()
-}
-
-func TestReceiverDestroy(t *testing.T) {
- c, _ := e.NewContext()
- defer c.Destroy()
-
r := e.receivers["TestReceiver"]
if r == nil {
t.Fatalf("Receiver.Destroy(): Could not find defined receiver")
@@ -166,8 +159,7 @@ func TestReceiverDestroy(t *testing.T) {
t.Errorf("Receiver.Destroy(): Did not set internal fields to `nil`")
}
- // Attempting to destroy a receiver twice should be a no-op.
- r.Destroy()
+ c.Destroy()
}
func TestReceiverEnd(t *testing.T) { |
GDB isn't super helpful as I'm not using a PHP7 library with debug enabled, but here it is anyway:
|
Looks like /* 15. Free Willy (here be crashes) */
zend_interned_strings_deactivate(); My educated guess is that https://github.com/php/php-src/blob/master/Zend/zend_API.c#L2726, which is what is called from lowercase_name = zend_new_interned_string(lowercase_name); Maybe this part of the PHP / Zend API wasn't mean to be used that way? |
Another observation: Calling Engine.Define before a context has been created also causes a segfault: package main
import (
"fmt"
php "github.com/deuill/go-php"
"os"
)
type Watcher struct {
Path string
}
func NewWatcher(args []interface{}) interface{} {
if len(args) > 0 {
if path, ok := args[0].(string); ok {
return &Watcher{path}
}
}
return nil
}
func main() {
engine, err := php.New()
if err != nil {
fmt.Printf("Could not create a new engine: %v\n", err)
return
}
defer engine.Destroy()
// Move the next two lines *below* the context creation to make it work
engine.Define("Watcher", NewWatcher)
fmt.Printf("Engine %+v\n", engine)
context, err := engine.NewContext()
if err != nil {
fmt.Printf("Could not create a new context: %v\n", err)
return
}
defer context.Destroy()
context.Output = os.Stdout
context.Eval("var_dump(new Watcher('Test'));")
} GDB:
The code at receiver.c:163 is: INIT_CLASS_ENTRY_EX(tmp, name, strlen(name), NULL); ...which expands to https://github.com/php/php-src/blob/master/Zend/zend_API.h#L188-L193: #define INIT_CLASS_ENTRY_EX(class_container, class_name, class_name_len, functions) \
{ \
memset(&class_container, 0, sizeof(zend_class_entry)); \
class_container.name = zend_string_init_interned(class_name, class_name_len, 1); \
class_container.info.internal.builtin_functions = functions; \
} Looks like class entries are not meant to be created before php_request_startup(); and not meant to be used after php_request_shutdown() |
From my point of view, this leaves us with the following options:
|
Good insight, thanks! I'll delve a bit deeper in these crashes, since the code around method receiver bindings is somewhat terrible. From what I can tell, ZEND_API void zend_interned_strings_deactivate(void)
{
zend_hash_destroy(&CG(interned_strings));
} Initializing a class defines a couple of interned strings, including the class name itself: #define INIT_OVERLOADED_CLASS_ENTRY_EX(class_container, class_name, class_name_len, functions, handle_fcall, handle_propget, handle_propset, handle_propunset, handle_propisset) \
{ \
zend_string *cl_name; \
cl_name = zend_string_init(class_name, class_name_len, 1); \
class_container.name = zend_new_interned_string(cl_name); \
INIT_CLASS_ENTRY_INIT_METHODS(class_container, functions, handle_fcall, handle_propget, handle_propset, handle_propunset, handle_propisset) \
} As well as, as you mentioned, the name used for registering the class against the compiler globals: static zend_class_entry *do_register_internal_class(zend_class_entry *orig_class_entry, uint32_t ce_flags) /* {{{ */
{
zend_class_entry *class_entry = malloc(sizeof(zend_class_entry));
zend_string *lowercase_name = zend_string_alloc(ZSTR_LEN(orig_class_entry->name), 1);
...
zend_str_tolower_copy(ZSTR_VAL(lowercase_name), ZSTR_VAL(orig_class_entry->name), ZSTR_LEN(class_entry->name));
lowercase_name = zend_new_interned_string(lowercase_name);
zend_hash_update_ptr(CG(class_table), lowercase_name, class_entry);
zend_string_release(lowercase_name);
return class_entry;
} The, ZEND_API void zend_interned_strings_init(void)
{
...
zend_init_interned_strings_ht(&interned_strings_permanent, 1);
zend_new_interned_string = zend_new_interned_string_permanent;
...
} However, the handler switches during ZEND_API void zend_interned_strings_switch_storage(void)
{
if (interned_string_copy_storage) {
interned_string_copy_storage();
}
zend_new_interned_string = interned_string_request_handler;
}
static zend_string *zend_new_interned_string_request(zend_string *str)
{
zend_string *ret;
if (ZSTR_IS_INTERNED(str)) {
return str;
}
/* Check for permanent strings, the table is readonly at this point. */
ret = zend_interned_string_ht_lookup(str, &interned_strings_permanent);
if (ret) {
zend_string_release(str);
return ret;
}
ret = zend_interned_string_ht_lookup(str, &CG(interned_strings));
if (ret) {
zend_string_release(str);
return ret;
}
/* Create a short living interned, freed after the request. */
ret = zend_add_interned_string(str, &CG(interned_strings), 0);
return ret;
} Sigh... The However, I now believe this may be due to my misunderstanding on how PHP's process model operates when not built with thread/ZTS support: So most things operate at the RINIT level anyways, since different requests aren't supposed to run concurrently. I'll run some tests and see about perhaps moving |
This happens when running tests:
Some research shows that is has to do with accessing receivers after a context is destroyed.
The text was updated successfully, but these errors were encountered: