Edit report at https://bugs.php.net/bug.php?id=71115&edit=1
ID: 71115
Updated by: krakjoe@php.net
Reported by: bobby dot mihalca at touchtech dot ro
Summary: data race on auto globals names and internal
functions names refcount
Status: Feedback
Type: Bug
Package: Reproducible crash
Operating System: Any
PHP Version: 7.0.0
Assigned To: ab
Block user comment: N
Private report: N
New Comment:
Persistent strings still use a refcount, they are just allocated using system memory rather than
zend heap.
There is nothing about using system memory that means they should actually persist.
Previous Comments:
------------------------------------------------------------------------
[2015-12-14 17:51:31] bobby dot mihalca at touchtech dot ro
It's a custom sapi based on a civetweb and while i can't publicly share the code, i can
send you a copy.
Just need a bit of time to rip it out of the project.
------------------------------------------------------------------------
[2015-12-14 16:53:01] ab@php.net
@bobby, could you please show the code reproducing the crash? It is so, that such global names are
allocated persistently and this is addressed in the GC info. So they should not be freed by GC and
persist all the process life time, while not sure it will go through it.
Currently the TS SAPI I work quite often is Apache, and neither Windows nor Linux show any crashes
or issues with dozens of threads. Of course it is not bug free. But exactly for this reason @bobby
I'd ask you to please show a piece of code to reproduce and debug the behavior you describe.
What SAPI is it? I can see a couple of places that could be improved, but without some real
reproducer it might be just a theory of mine.
Thanks.
------------------------------------------------------------------------
[2015-12-14 14:20:47] laruence@php.net
weltingï¼ what do you think? Make them interned seems okey,but not sure if there is similar
issues out of this?
------------------------------------------------------------------------
[2015-12-14 14:14:03] bobby dot mihalca at touchtech dot ro
Exclusive locking the sapi startup was also my first attempt.
Unfortunately valgring's helgrind would still detect a data race in zend_lookup_class_ex at
ZVAL_STR_COPY(&fcall_info.function_name, EG(autoload_func)->common.function_name);
While exclusive locking during startup stop always segfaulting, the data race is still there and
could segfault, is just less probable.
IMHO my fix is quite correct because:
1. names are already persistent allocated and HAVE TO live as long as the sapi
2. AFAIK php uses copy on write for strings, so even is a name is modified a new string will be
created
3. names don't change, _SERVER, _GET, _POST, fopen, fclose all keep their name
4. since interned flag is removed php_module_shutdown would free the strings and everything should
be ok (assuming that request are active at sapi shutdown, witch i guess is true)
However i think my sample does not cover dynamic loaded extensions, for my use case all modules are
static linked, so it works well.
A nice bonus of this fix is a increase from ~35k req/sec to ~50k req/sec due to lower startup cost.
Since all the names are interned even under ZTS it skips hundreds of (pointless) emalloc to copy
them and just uses existing interned zend_string*
If i did not miss somthing, a proper fix could integrate the changes in
php_module_startup/php_module_shutdown and dynamic module loading/unloading.
------------------------------------------------------------------------
[2015-12-14 13:18:13] krakjoe@php.net
This is definitely a problem ...
pthreads uses critical sections around startup/shutdown of user threads to work around the problem
... I'd like to remove those sections ...
I'm not sure what the proper fix should look like though, disabling refcounts on those strings
will hide internal programmer error (leaks).
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=71115
--
Edit this bug report at https://bugs.php.net/bug.php?id=71115&edit=1