Zend/zend_hash.c(919) : ht=0x... is already destroyed #82

Closed
bwoebi opened this Issue Mar 21, 2013 · 36 comments

Projects

None yet

3 participants

@bwoebi
Collaborator
bwoebi commented Mar 21, 2013

This is not a bug report as I think it is when I do something wrong.

But I don't know what exactly is causing this and how to work around it.

I see this error not too rarely. I'd like to see somewhere an explanation what this problem cause is and how to work around it — favourably also in the docs.

@krakjoe
Owner
krakjoe commented Mar 22, 2013

Sounds a bit buggy to me, is there any reproduce script at all ??

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013

Ok… might be really a bug…

<?php

class Data extends Stackable {
        public function run () {}
}

class Stack extends Stackable {
        public function run () {}

        public $instances;
        public $field;

        public function addInstance ($instance) {
                $this->instances = array_merge($this->instances?:[], [$instance]);
        }
}

class Th extends Thread {
        public $data;
        public $stack;

        // If I comment the construct function out, it segfaults
        /* Backtrace:

#0  0x000000010091f494 in zend_hash_find (ht=0x102910970, arKey=0x1022bd9e8 "addInstance", nKeyLength=11, pData=0x102780758) at zend_hash.c:924
#1  0x00000001017c85a9 in pthreads_modifiers_get (modifiers=0x102910970, method=0x1022bd9e8 "addInstance", tsrm_ls=0x102800000) at modifiers.c:106
#2  0x00000001017c97d8 in pthreads_get_method (pobject=0x102785588, method=0x1022bd9e8 "addInstance", methodl=11, key=0x1022bd2f8, tsrm_ls=0x102800000) at handlers.c:285
#3  0x000000010098ff5e in ZEND_INIT_METHOD_CALL_SPEC_VAR_CONST_HANDLER (execute_data=0x1027854e0, tsrm_ls=0x102800000) at zend_vm_execute.h:15356
#4  0x000000010095a204 in execute_ex (execute_data=0x1027854e0, tsrm_ls=0x102800000) at zend_vm_execute.h:356
#5  0x000000010095a2fe in zend_execute (op_array=0x102a5cd80, tsrm_ls=0x102800000) at zend_vm_execute.h:381
#6  0x00000001008e7a26 in zend_call_function (fci=0x102780db0, fci_cache=0x102780d88, tsrm_ls=0x102800000) at zend_execute_API.c:941
#7  0x00000001017cc462 in pthreads_routine (arg=0x102612650) at object.c:708
#8  0x00007fff99a80742 in _pthread_start ()
#9  0x00007fff99a6d181 in thread_start ()

        */

        public function __construct ($data) {
                $this->data = $data;
        }

        public function run () {
                $this->stack = new Stack();
                $stack = &$this->stack;
                $stack->field = "string"; // Warning: pthreads failed to write member Stack::$field
                var_dump($this->stack); // Does NOT show that $field would have been saved...
                $this->stack->addInstance($this); // Zend/zend_hash.c(919) : ht=0x... is inconsistent
        }
}

$t = new Th(new Data);
$t->start();
$t->join();
@krakjoe
Owner
krakjoe commented Mar 22, 2013

Yeah, thats a bug ....

<?php

class Data extends Stackable {
        public function run () {}
}

class Stack extends Stackable {
        public function run () {}

        public $instances;
        public $field;

        public function addInstance ($instance) {
                $this->instances = array_merge($this->instances?:[], [$instance]);
        }
}

class Th extends Thread {
        public $data;
        public $stack;

        // If I comment the construct function out, it segfaults
        /* Backtrace:

#0  0x000000010091f494 in zend_hash_find (ht=0x102910970, arKey=0x1022bd9e8 "addInstance", nKeyLength=11, pData=0x102780758) at zend_hash.c:924
#1  0x00000001017c85a9 in pthreads_modifiers_get (modifiers=0x102910970, method=0x1022bd9e8 "addInstance", tsrm_ls=0x102800000) at modifiers.c:106
#2  0x00000001017c97d8 in pthreads_get_method (pobject=0x102785588, method=0x1022bd9e8 "addInstance", methodl=11, key=0x1022bd2f8, tsrm_ls=0x102800000) at handlers.c:285
#3  0x000000010098ff5e in ZEND_INIT_METHOD_CALL_SPEC_VAR_CONST_HANDLER (execute_data=0x1027854e0, tsrm_ls=0x102800000) at zend_vm_execute.h:15356
#4  0x000000010095a204 in execute_ex (execute_data=0x1027854e0, tsrm_ls=0x102800000) at zend_vm_execute.h:356
#5  0x000000010095a2fe in zend_execute (op_array=0x102a5cd80, tsrm_ls=0x102800000) at zend_vm_execute.h:381
#6  0x00000001008e7a26 in zend_call_function (fci=0x102780db0, fci_cache=0x102780d88, tsrm_ls=0x102800000) at zend_execute_API.c:941
#7  0x00000001017cc462 in pthreads_routine (arg=0x102612650) at object.c:708
#8  0x00007fff99a80742 in _pthread_start ()
#9  0x00007fff99a6d181 in thread_start ()

        */

        public function __construct ($data) {
                $this->data = $data;
        }

        public function run () {
                $this->stack = ($s=new Stack());
                $stack = &$this->stack;
                $stack->field = "string"; // Warning: pthreads failed to write member Stack::$field
                var_dump($this->stack); // Does NOT show that $field would have been saved...
                $this->stack->addInstance($this); // Zend/zend_hash.c(919) : ht=0x... is inconsistent
        }
}

$t = new Th(new Data);
$t->start();
$t->join();

That will allow it to run without error, or should ... I will look into it today ...

On a side note, = &$ is pointless on a pthreads object, everything you manipulate in any context is only a reference in the first instance, $ = $ does not result in a copy of the whole object, just a copy of the address anyway ...

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013
$this->stack = ($s=new Stack());

This works as long as you do this in methods/functions which are left at some point. (and don't pass a variable by reference)

&$ is pointless on every object, yes. And sometimes I don't know myself why I do this...

@krakjoe krakjoe added a commit that closed this issue Mar 22, 2013
@krakjoe fix #82 11fddd3
@krakjoe krakjoe closed this in 11fddd3 Mar 22, 2013
@krakjoe
Owner
krakjoe commented Mar 22, 2013

That should do it ...

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013

It isn't completely fixed yet.

I'll analyze it and send you a new reproduce script (in a 45 minutes I think). But at least this special case works now, yes.

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013
<?php

class Stack extends Stackable {
        public function run () {}

        public $instances;
        public $field;

        public function addInstance ($instance) {
                $this->instances = array_merge($this->instances?:[], [$instance]);
        }
}

class Th extends Thread {
        public $stack;

        public function run () {
                $this->stack = new Stack();
                $stack = $this->stack;
                $stack->addInstance(new SubTh);
                $thread = $stack->instances[count($stack->instances) - 1];
                $thread->start(); // Zend/zend_hash.c(919) : ht=0x... is inconsistent
        }
}

class SubTh extends Thread {
        public function run () {
                sleep(1);
        }
}

$t = new Th();
$t->start();
$t->join();

This is crashing with the inconstistent error... There is no warning it could not be serialized etc., so I suppose the thread can be stored in the stackable...

@krakjoe
Owner
krakjoe commented Mar 22, 2013

Ok, this is the first properly strange bug ... check it out ....

Starting program: /usr/local/bin/php-zts bug.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
warning: the debug information found in "/usr/lib/debug/usr/lib64/libfreebl3.so.debug" does not match "/lib64/libfreebl3.so" (CRC mismatch).


Breakpoint 2, pthreads_base_ctor (base=0x12bb1a0, entry=0x7fffed489f60, tsrm_ls=0x101f6e0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) n
365                     zend_object_std_init(&base->std, entry TSRMLS_CC);
(gdb) 
378                     object_properties_init(&(base->std), entry);
(gdb) 
381                     base->cls = tsrm_ls;
(gdb) p base->std.ce
$5 = (zend_class_entry *) 0x7fffed489f60
(gdb) p *base->std.ce
$6 = {type = 2 '\002', name = 0x7ffff7fbd2a0 "Th", name_length = 2, parent = 0x11f1790, refcount = 1, ce_flags = 1048576, function_table = {
    nTableSize = 16, nTableMask = 15, nNumOfElements = 15, nNextFreeElement = 0, pInternalPointer = 0x7fffed487b38, pListHead = 0x7fffed487b38, 
    pListTail = 0x7fffed48d488, arBuckets = 0x7fffed48c968, pDestructor = 0x8d0c54 <zend_function_dtor>, persistent = 0 '\000', nApplyCount = 0 '\000', 
    bApplyProtection = 0 '\000', inconsistent = 0}, properties_info = {nTableSize = 8, nTableMask = 7, nNumOfElements = 1, nNextFreeElement = 0, 
    pInternalPointer = 0x7fffed48a260, pListHead = 0x7fffed48a260, pListTail = 0x7fffed48a260, arBuckets = 0x7ffff7fbdaf8, 
    pDestructor = 0x89e446 <zend_destroy_property_info>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', inconsistent = 0}, 
  default_properties_table = 0x7fffed4899d8, default_static_members_table = 0x0, static_members_table = 0x0, constants_table = {nTableSize = 8, 
    nTableMask = 0, nNumOfElements = 0, nNextFreeElement = 0, pInternalPointer = 0x0, pListHead = 0x0, pListTail = 0x0, arBuckets = 0x10196b0, 
    pDestructor = 0x8dda1d <_zval_ptr_dtor_wrapper>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', inconsistent = 0}, 
  default_properties_count = 1, default_static_members_count = 0, constructor = 0x0, destructor = 0x0, clone = 0x0, __get = 0x0, __set = 0x0, 
  __unset = 0x0, __isset = 0x0, __call = 0x0, __callstatic = 0x0, __tostring = 0x0, serialize_func = 0x0, unserialize_func = 0x0, iterator_funcs = {
    funcs = 0x0, zf_new_iterator = 0x0, zf_valid = 0x0, zf_current = 0x0, zf_key = 0x0, zf_next = 0x0, zf_rewind = 0x0}, 
  create_object = 0x7ffff14d8305 <pthreads_thread_ctor>, get_iterator = 0, interface_gets_implemented = 0, get_static_method = 0, 
  serialize = 0x7ffff14d8e49 <pthreads_internal_serialize>, unserialize = 0x7ffff14d8ef1 <pthreads_internal_unserialize>, interfaces = 0x0, 
  num_interfaces = 0, traits = 0x0, num_traits = 0, trait_aliases = 0x0, trait_precedences = 0x0, info = {user = {
      filename = 0x7ffff7fbbdf0 "/usr/src/pthreads/bug.php", line_start = 14, line_end = 24, doc_comment = 0x0, doc_comment_len = 0}, internal = {
      builtin_functions = 0x7ffff7fbbdf0, module = 0x180000000e}}}
(gdb) c
Continuing.

Breakpoint 1, pthreads_start (thread=0x12bb1a0, tsrm_ls=0x101f6e0) at /usr/src/pthreads/src/object.c:474
474             int dostart = 0;
(gdb) c
Continuing.
[New Thread 0x7fffed486700 (LWP 8099)]
[Switching to Thread 0x7fffed486700 (LWP 8099)]

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cb9a0, entry=0x7fffecbb6b08, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) n
365                     zend_object_std_init(&base->std, entry TSRMLS_CC);
(gdb) 
378                     object_properties_init(&(base->std), entry);
(gdb) p *base->std.ce
$7 = {type = 2 '\002', name = 0x7fffecbb6da0 "Th", name_length = 2, parent = 0x11f1790, refcount = 1, ce_flags = 1048576, function_table = {
    nTableSize = 16, nTableMask = 15, nNumOfElements = 15, nNextFreeElement = 0, pInternalPointer = 0x7fffecbb6ea0, pListHead = 0x7fffecbb6ea0, 
    pListTail = 0x7fffecbb8c00, arBuckets = 0x7fffecbb80e0, pDestructor = 0x8d0c54 <zend_function_dtor>, persistent = 0 '\000', nApplyCount = 0 '\000', 
    bApplyProtection = 0 '\000', inconsistent = 0}, properties_info = {nTableSize = 8, nTableMask = 7, nNumOfElements = 1, nNextFreeElement = 0, 
    pInternalPointer = 0x7fffecbb8e08, pListHead = 0x7fffecbb8e08, pListTail = 0x7fffecbb8e08, arBuckets = 0x7fffecbb6e00, 
    pDestructor = 0x7ffff14d34ad <pthreads_preparation_property_info_dtor>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', 
    inconsistent = 0}, default_properties_table = 0x7fffecc84d90, default_static_members_table = 0x0, static_members_table = 0x0, constants_table = {
    nTableSize = 8, nTableMask = 0, nNumOfElements = 0, nNextFreeElement = 0, pInternalPointer = 0x0, pListHead = 0x0, pListTail = 0x0, 
    arBuckets = 0x10196b0, pDestructor = 0x8dda1d <_zval_ptr_dtor_wrapper>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', 
    inconsistent = 0}, default_properties_count = 1, default_static_members_count = 0, constructor = 0x0, destructor = 0x0, clone = 0x0, __get = 0x0, 
  __set = 0x0, __unset = 0x0, __isset = 0x0, __call = 0x0, __callstatic = 0x0, __tostring = 0x0, serialize_func = 0x0, unserialize_func = 0x0, 
  iterator_funcs = {funcs = 0x0, zf_new_iterator = 0x0, zf_valid = 0x0, zf_current = 0x0, zf_key = 0x0, zf_next = 0x0, zf_rewind = 0x0}, 
  create_object = 0x7ffff14d8305 <pthreads_thread_ctor>, get_iterator = 0, interface_gets_implemented = 0, get_static_method = 0, 
  serialize = 0x7ffff14d8e49 <pthreads_internal_serialize>, unserialize = 0x7ffff14d8ef1 <pthreads_internal_unserialize>, interfaces = 0x0, 
  num_interfaces = 0, traits = 0x0, num_traits = 0, trait_aliases = 0x0, trait_precedences = 0x0, info = {user = {
      filename = 0x7ffff7fbbdf0 "/usr/src/pthreads/bug.php", line_start = 14, line_end = 24, doc_comment = 0x0, doc_comment_len = 0}, internal = {
      builtin_functions = 0x7ffff7fbbdf0, module = 0x180000000e}}}
(gdb) c
Continuing.

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cbc20, entry=0x7fffecbb5078, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) n
365                     zend_object_std_init(&base->std, entry TSRMLS_CC);
(gdb) 
378                     object_properties_init(&(base->std), entry);
(gdb) n
381                     base->cls = tsrm_ls;
(gdb) p *base->std.ce
$8 = {type = 2 '\002', name = 0x7fffecc84f40 "Stack", name_length = 5, parent = 0x11f4240, refcount = 1, ce_flags = 1048576, function_table = {
    nTableSize = 16, nTableMask = 15, nNumOfElements = 10, nNextFreeElement = 0, pInternalPointer = 0x7fffecbb5310, pListHead = 0x7fffecbb5310, 
    pListTail = 0x7fffecbb6560, arBuckets = 0x7fffecc78660, pDestructor = 0x8d0c54 <zend_function_dtor>, persistent = 0 '\000', nApplyCount = 0 '\000', 
    bApplyProtection = 0 '\000', inconsistent = 0}, properties_info = {nTableSize = 8, nTableMask = 7, nNumOfElements = 2, nNextFreeElement = 0, 
    pInternalPointer = 0x7fffecbb6768, pListHead = 0x7fffecbb6768, pListTail = 0x7fffecbb68b8, arBuckets = 0x7fffecc77c28, 
    pDestructor = 0x7ffff14d34ad <pthreads_preparation_property_info_dtor>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', 
    inconsistent = 0}, default_properties_table = 0x7fffecc84ed0, default_static_members_table = 0x0, static_members_table = 0x0, constants_table = {
    nTableSize = 8, nTableMask = 0, nNumOfElements = 0, nNextFreeElement = 0, pInternalPointer = 0x0, pListHead = 0x0, pListTail = 0x0, 
    arBuckets = 0x10196b0, pDestructor = 0x8dda1d <_zval_ptr_dtor_wrapper>, persistent = 0 '\000', nApplyCount = 0 '\000', bApplyProtection = 0 '\000', 
    inconsistent = 0}, default_properties_count = 2, default_static_members_count = 0, constructor = 0x0, destructor = 0x0, clone = 0x0, __get = 0x0, 
  __set = 0x0, __unset = 0x0, __isset = 0x0, __call = 0x0, __callstatic = 0x0, __tostring = 0x0, serialize_func = 0x0, unserialize_func = 0x0, 
  iterator_funcs = {funcs = 0x0, zf_new_iterator = 0x0, zf_valid = 0x0, zf_current = 0x0, zf_key = 0x0, zf_next = 0x0, zf_rewind = 0x0}, 
  create_object = 0x7ffff14d843b <pthreads_stackable_ctor>, get_iterator = 0, interface_gets_implemented = 0, get_static_method = 0, 
  serialize = 0x7ffff14d8e49 <pthreads_internal_serialize>, unserialize = 0x7ffff14d8ef1 <pthreads_internal_unserialize>, interfaces = 0x0, 
  num_interfaces = 0, traits = 0x0, num_traits = 0, trait_aliases = 0x0, trait_precedences = 0x0, info = {user = {
      filename = 0x7ffff7fbbdf0 "/usr/src/pthreads/bug.php", line_start = 3, line_end = 12, doc_comment = 0x0, doc_comment_len = 0}, internal = {
      builtin_functions = 0x7ffff7fbbdf0, module = 0xc00000003}}}
(gdb) c
Continuing.

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cc080, entry=0x7fffecbb5078, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) c
Continuing.

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cc310, entry=0x7fffecbb8fd8, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) c
Continuing.

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cc5a0, entry=0x7fffecbb8fd8, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) c
Continuing.

Breakpoint 2, pthreads_base_ctor (base=0x7fffe80cc310, entry=0x7fffecbb8fd8, tsrm_ls=0x7fffe80008c0) at /usr/src/pthreads/src/object.c:364
364             if (base) {
(gdb) c
Continuing.
/usr/src/php-zts/Zend/zend_hash.c(919) : ht=0x7fffe80cc5a0 is already destroyed
[Thread 0x7fffed486700 (LWP 8099) exited]
warning: Temporarily disabling breakpoints for unloaded shared library "/opt/php-zts/lib/php/extensions/debug-zts-20121212/pthreads.so"
[Inferior 1 (process 8098) exited normally]

2nd from last base_ctor has base address the same as the address in the error ... definitely this is a wierd bug ...

I am on it though, thanks for the report :)

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013

Okay, sounds strange, but I don't think I'd be able to help as I don't exactly know how pthreads works, step by step...

Good luck :)

@krakjoe
Owner
krakjoe commented Mar 22, 2013
<?php
class Instances extends Stackable {
    public function run(){}
}

class Stack extends Stackable {
    public function run () {}

    public $instances;
    public $field;

    public function __construct(){
        $this->instances = new Instances();
    }

    public function addInstance ($instance) {
        return ($this->instances[] = $instance);
    }

    public function getLength() {
        return count($this->instances);
    }
}

class Th extends Thread {
    public $stack;

    public function run () {
        $this->stack = new Stack();
        $stacked = $this->stack->addInstance(
            new SubTh
        );
        $stacked->start();
        $stacked->join();
    }
}

class SubTh extends Thread {
    public function run () {
            sleep(1);
    }
}

$t = new Th();
$t->start();
$t->join();

var_dump($t->stack->instances);

I'll be finished for the day soon, bit busy this evening, but wanted to show you this before I go .... this is how I would have written it, I will find the bugs, but some of the things you are doing are suboptimal so thought I'd provide a good, working example. The bugs we have here steam from the detection of origin of an object, but so long as you define those origins well, like in my example, you shouldn't hit this sort of error. There is no real problem with threads creating threads and all sharing objects as you know from examples and no doubt your own work, but somewhere there has to be a reference to every object that you want to reference ...

I'll keep this updated with my progress, I don't expect I'll find a solution quickly, I have a rough of why this is happening already ...

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013

What do you name a well-defined origin?

I hope this'll be fixed fast. What I am doing is instantiating the object in one method and start it in another. So I really need to access it via this array.

@krakjoe
Owner
krakjoe commented Mar 22, 2013

Yeah, you can do that, and you can use arrays, but it's sub-optimal because you have to keep rebuilding the array with array_merge, you can access the indexes of any pthreads object like you can an array, and they do not require copying on each edit, or un-serializing on each reference.

By a well defined origin, I mean if you pass an object into a method with no other references ( ie. call(new Obj()) ), then have the method keep a reference. I notice you have created the stack inside the first thread, which is fine in the example, but in normal code you might want to access that from the main process again, which you won't be able to do by the time you have joined and destroyed that thread.

@olekukonko
Contributor

@krakjoe Tested your latest code .. and it still crashed ...

but it's sub-optimal because you have to keep rebuilding the array with array_merge this is related to #79 and similar issues would keep coming up with time.

Best of luck fixing this ...

@bwoebi
Collaborator
bwoebi commented Mar 22, 2013

@olekukonko It doesn't crash, it outputs NULL. Are you sure you use the right code…?

@krakjoe
Owner
krakjoe commented Mar 23, 2013

@olekukonko can you update to git master, that's what I am working with.

There is no issue if you follow my advice, an array is not a suitable structure for multiple threads to manipulate and the Zend engine prohibits it from occurring even if you try. You can use arrays, by merging, or you can follow best practice and write your collections properly.

The problem is here, that pthreads treats an unserialized object representing a pthreads object differently than it treats the original variable declared as the pthreads object. Writing it to an array, or another pthreads object results in loosing the original variable, which, you can't really do right now. There are some inconsistencies internally, namely use of PTHREADS_IS_NOT_CONNECTION over PTHREADS_IN_CREATOR and vice-versa, but even when they are corrected it does not yield the desired result, it will show an error about starting a thread created in another context ... I'll have a think about what to do ... but you can still avoid, perfectly safely, this error.

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

By the way I haven't understood yet when I shall use array_merge and when the direct []= assign operator?

Why don't you, when you start a thread, wait (internally) for the main thread and start it there? As far as I see, it doesn't change anything when you start it from main thread?

@krakjoe
Owner
krakjoe commented Mar 23, 2013

You should never use array merge on member data, data shared among contexts should always be derived from a pthreads object.

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

Okay :) I also didn't know until a few hours ago that a stackable internally implements arrayaccess...

can you implement a pop, push, unpop and unshift method (internally; I know it's feasible in userland, but surely it'll be more performant to do this internally) Or is there some possibility to cast to array etc.? (I'm not really sure..., but when I see the last edits...?)

@krakjoe
Owner
krakjoe commented Mar 23, 2013

The issue is, these methods will not be enough in the majority of circumstances to get you desirable behaviour on their own. When you are working with a collection of data that multiple contexts can manipulate, you want to handle the data as a set (or maybe not, that's exactly the point, the power is with you in userland), not each member on it's own (or maybe so). It will be better that the (very easy) logic for those methods is integrated into the design of the collection.

All objects also implement Countable, so you don't have to maintain counters.

Casting can be used to create a normal array from an objects properties, BUT, they are separated from the properties of the object ( changes have no effect ), and the entire object is locked up while they are copied. This might be suitable for some quick collection implementations, it is mostly just shorthand.

We have to start experimenting with a set of classes that implement some different sorts of collections/queues/lists, various locking/copying/referencing schemes ... there is just one last thing to do before we can really do that, we need a method to get the keys, I'll do it in the next few days ...

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

What would be now the best way for a jobqueue?

I have an stackable "array" with [$callback, $args] to be passed to call_user_func_array.

Now I want to remove always the first element of the array (array_shift) and return it.

Do I have to write code like:

class ArrayStack extends Stackable {
    public function shift () {
        $count = count($this);
        $retval = $this[0];
        for ($i = 1; $i < $count; $i++)
            $this[$i - 1] = $this[$i];
        unset($this[$count - 1]);
        return $retval;
    }

    public function run () {}
}

Surely, this will work, but is this the optimal way...?

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

These methods are maybe not enough, but they are the functions I use most when playing around with arrays.

@krakjoe
Owner
krakjoe commented Mar 23, 2013

It depends doesn't it, on your intended usage across the application. Maybe shift should be simply protected, maybe it should use this->lock, maybe it should check some other member of the class to decide how to behave before maybe locking or not ... this is exactly what I mean, there is no generic way to implement this that will suit everyone.

Of course, eventually all of this should be in C. My suggestion is, as outlined in issue reports, that we gather and develop a set of classes that cover all the reasonable angles, just like java do, when those implementation are real world tested, debugged and working well, I will invest the time in rewriting them in C. But there is no harm in the development stage occuring in PHP.

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

Humm, what I hate in Java IS that there is a class for every microthing. I don't want some special classes, but only one basic implementation of the basic functions. When you want locking before everything, you can make a __call magic method and then lock it.
I mean only the basic functionality in C and the decorations you're free to do in magic methods...

@krakjoe
Owner
krakjoe commented Mar 23, 2013

Well it's kind of surprising that you have knowledge of concurrency in java and yet can imagine one implementation to be enough, I think maybe you take much of it's functionality for granted. We are working with bare bones, like your example code there for shift(), if you execute that, you will run into problems, lots of them, even though it seems sane it is nothing like thread safe.

Eventually I will have to write this stuff on my own I guess, if no one is going to jump on board, but I won't be hacking together any methods in the current API in the mean time. It's better that you implement what you require in userland from your collections than I restrict the API hugely by deciding what is possible.

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

This code isn't thread-safe. right. For example appending in the one thread after counting the array size in another thread. I am not the big java developer, but I've already played around with it, resulting in code with much synchronized-keywords before function declarations. But I forget that I have to do this in PHP also (it's so unusal there... ;)). When I develop a jobqueue for example I don't notice this as I am the only tester. Problem is when such code is found to be good and put in production... So, it needs ONE thread-safe implementation. In the rare cases where a shared array is passed between context and should be used in an unsafe way, the developer will be able to implement the little function himself.

Btw. I don't want you to restrict the API, but only write a primary, thread safe, possibility everyone can use without problems. Every user to whom the methods aren't good enough can feel free to overload them?

@krakjoe
Owner
krakjoe commented Mar 23, 2013

Safety is subjective though isn't it, all of the manipulation routines provided by java are safe, so long as you use them as intended. Same with pthreads right now, everything you do is atomic, because I don't provide ways to manipulate the data as a set, both atomic and non-atomic set operations can be implemented in userland. If I implement anything more, then every implementation relying on it will have to follow my rules which would have to be the most restrictive version imaginable for it to be "stable" ... so hopefully you see the dilema ... these are not problems we should look to solve within pthreads, they really really aren't ...

@bwoebi
Collaborator
bwoebi commented Mar 23, 2013

Well, you may implement the functions with an optional last parameter $atomic = true?

When someone doesn't like your implementation his may use his own implementation — he's free to choose your restrictions or not? But yes, this doesn't have the highest priority. (Or don't I understand what you mean?!)

@krakjoe
Owner
krakjoe commented Mar 23, 2013

I think you get it, I just prefer to have a collection of well defined objects, and well developed ideas, than working around the problems with a few (flexible) methods on every object.

There is not much expertise in this field when it comes to PHP programmers, I don't want to go down the PHP road of adding methods with optional parameters to work around problems, I don't believe it will serve us well in the future. I would rather have, and users will benefit hugely from, a collection of well defined and well documented objects for managing multi-threaded collections, I am determined this will happen, and is the best solution long term.

@bwoebi
Collaborator
bwoebi commented Mar 24, 2013

How do you think you want to proceed to implement these collection? make some survey first what's really needed? Or implement without asking someone what you think is the best...?

@krakjoe
Owner
krakjoe commented Mar 24, 2013

I was hoping that I wouldn't have to be the lead for the next part, I was hoping the people using it would take the lead and I would help out and fix the bugs you find in the process...

I am not really sure ...

@olekukonko
Contributor

I think we can start with more examples , use cases and work around to this issues .. Also reflect this concept in the documentation .....

@bwoebi
Collaborator
bwoebi commented Mar 24, 2013

Then, if we have an implementation of something, always open a new issue or where put it?

p.s. $stackablearray->(un)lock() is the right way for thread safety, right?

@krakjoe
Owner
krakjoe commented Mar 24, 2013

That controls the lock on the object store.

There is also protected (I think you mentioned you knew about this), akin to java's synchronized for methods that should be executed exclusively - v helpful for the implementation of collections.

Additionally, you can use raw mutex/conditions to gain more precise control over locking, if you feel that way inclined, just be sure to destroy them when finished with else you will create leaks (that will not be reported as leaks by zend)

@krakjoe
Owner
krakjoe commented Mar 24, 2013

I will start a git so we can pool resources there ... anyone who becomes an active contributor will get direct access without pr's ...

http://github.com/krakjoe/concurrent

@bwoebi
Collaborator
bwoebi commented Mar 24, 2013

I wanted to now the best way to lock such methods like shift generally. I know that there are Conds Mutexes, synchronized, lock etc. But what's the best, most efficient? on a stackable which serves only as an array?

@krakjoe
Owner
krakjoe commented Mar 24, 2013

that'd be protected

I have started out writing an ICollection interface, if we could now move this discussion to the concurrent repos and start collaborating there, that'd be good ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment