krakjoe / parallel

A succinct parallel concurrency API for PHP8
Other
1.45k stars 95 forks source link

segfault when calling a function defined in global scope #317

Open rcmcdonald91 opened 1 month ago

rcmcdonald91 commented 1 month ago

Reproducer:

use parallel\Runtime;
function a() { return; }
(new Runtime)->run(function() { a(); });
ZEND_INIT_FCALL_SPEC_CONST_HANDLER () at /home/rcm/Projects/thing/php-src/Zend/zend_vm_execute.h:4044
4044                    fbc = Z_FUNC_P(func);
(gdb) bt 
#0  ZEND_INIT_FCALL_SPEC_CONST_HANDLER () at /home/rcm/Projects/thing/php-src/Zend/zend_vm_execute.h:4044
#1  execute_ex (ex=0x7fffec004ef0) at /home/rcm/Projects/thing/php-src/Zend/zend_vm_execute.h:58907
#2  0x00007ffff7fab0eb in php_parallel_scheduler_run (frame=frame@entry=0x7ffff3a14020, runtime=0x7ffff545b280)
    at /home/rcm/Projects/thing/parallel/src/scheduler.c:331
#3  0x00007ffff7fab63f in php_parallel_thread (arg=0x7ffff545b280) at /home/rcm/Projects/thing/parallel/src/scheduler.c:501
#4  0x00007ffff7a98ded in ?? () from /usr/lib/libc.so.6
#5  0x00007ffff7b1c0dc in ?? () from /usr/lib/libc.so.6
PHP 8.4.0-dev (cli) (built: Jul 21 2024 23:21:30) (ZTS)
Copyright (c) The PHP Group
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies

This probably shouldn't segfault.

realFlowControl commented 1 month ago

Hey there 👋

This is not expected to work, as we do not copy the function table on thread creation. It works if you enable OPcache, but still it should not and it is "just" a side effect, so please do not rely on it. Have a look at this test:

https://github.com/krakjoe/parallel/blob/5387f66f49a6b0393b44b7bf1a2748610f87feb1/tests/functional/003.phpt#L10-L18

https://github.com/krakjoe/parallel/blob/5387f66f49a6b0393b44b7bf1a2748610f87feb1/tests/functional/003.inc#L1-L2

The overall idea is that you need a bootstrap file, either using \parallel\bootstrap() or passing it to the \parallel\Runtime-constructor. This bootstrap file either defines the functions or is your autoloader to autoload classes and functions in that thread.

Hope this helps

rcmcdonald91 commented 1 month ago

Hey there 👋

This is not expected to work, as we do not copy the function table on thread creation. It works if you enable OPcache, but still it should not and it is "just" a side effect, so please do not rely on it. Have a look at this test:

https://github.com/krakjoe/parallel/blob/5387f66f49a6b0393b44b7bf1a2748610f87feb1/tests/functional/003.phpt#L10-L18

https://github.com/krakjoe/parallel/blob/5387f66f49a6b0393b44b7bf1a2748610f87feb1/tests/functional/003.inc#L1-L2

The overall idea is that you need a bootstrap file, either using \parallel\bootstrap() or passing it to the \parallel\Runtime-constructor. This bootstrap file either defines the functions or is your autoloader to autoload classes and functions in that thread.

Hope this helps

Thanks for the reply. Yep, I'm aware that this shouldn't work and shouldn't be utilized. I only mention it because I believe we can do better than seg-faulting and instead throw a fatal exception that is more useful for the average user.

realFlowControl commented 1 month ago

Ah okay, got it! I too think that "not seg-faulting" would be more user friendly 😄 I'll think about how we could fail nicer, would be nice if would just fail like when calling a function that does not exist:

Fatal error: Uncaught Error: Call to undefined function a() in ...
realFlowControl commented 1 month ago

This is btw. what is happening:

Process 2943 stopped
* thread #4, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010081fcfc php`ZEND_INIT_FCALL_SPEC_CONST_HANDLER(execute_data=0x0000000102a13020) at zend_vm_execute.h:3842:9
   3839         fname = (zval*)RT_CONSTANT(opline, opline->op2);
   3840         func = zend_hash_find_known_hash(EG(function_table), Z_STR_P(fname));
   3841         ZEND_ASSERT(func != NULL && "Function existence must be checked at compile time");
-> 3842         fbc = Z_FUNC_P(func);
   3843         if (EXPECTED(fbc->type == ZEND_USER_FUNCTION) && UNEXPECTED(!RUN_TIME_CACHE(&fbc->op_array))) {
   3844             init_func_run_time_cache(&fbc->op_array);
   3845         }
(lldb) bt
* thread #4, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010081fcfc php`ZEND_INIT_FCALL_SPEC_CONST_HANDLER(execute_data=0x0000000102a13020) at zend_vm_execute.h:3842:9
    frame #1: 0x00000001007be7e0 php`execute_ex(ex=0x0000000102a13020) at zend_vm_execute.h:57007:7
    frame #2: 0x00000001017494e8 parallel.so`php_parallel_scheduler_run(runtime=0x000000010185e140, frame=0x0000000102a13020) at scheduler.c:332:13
    frame #3: 0x0000000101748924 parallel.so`php_parallel_thread(arg=0x000000010185e140) at scheduler.c:506:9
    frame #4: 0x000000018c5baf94 libsystem_pthread.dylib`_pthread_start + 136
(lldb) p func
(zval *) NULL

The function seem to exists and not exist at the same time 😉 So it is definitely inconsistent state that should be cleaned if we can, although for the time being I can life with the two options:

I guess it should be fixable, but there are some more pressing issues open currently. I'll keep this open and might spend some time in the future on it.

realFlowControl commented 1 month ago

Might not be that easy after all.