On 13/09/13 22:10, Lazy wrote:
Hello internals,

I'm trying to fix deadlock in an ancient php 5.2.17, php hangs on
internal libc lock.
 From my understanding free is not safe to use in a signal handler, and
this seems to be the issue here.
No, it's not.

http://marc.info/?l=php-internals&m=121999390109071&w=2 seems to
address this issue but it's not present in 5.3 or later releases.

Very similar deadlock but with time() instead of free(),
https://bugs.php.net/bug.php?id=31749
Are you sure it's with time() ? I do see a free() in that call stack (and no time), as I would expect, as time() is required by POSIX.1-2004 to be Async-signal-safe.

Current php also uses free() in php_error_cb() and external
destructors in list_entry_destructor() aren't protected by
HANDLE_BLOCK_INTERRUPTIONS() (which seems to be a noop in fastcgi
mode), so I suspect 5.5 may also contain this deadlock.

main/main.c
php_error_cb()
     835     if (display) {
     836         if (PG(last_error_message)) {
     837             free(PG(last_error_message));
...                    ^^^^^^ deadlock if previous free was
interrupted by a timeout signal
     845         PG(last_error_type) = type;
     846         PG(last_error_message) = strdup(buffer);

I'm thinking about "fixing" this by leaking memory pointed by
PG(last_error_message) if php called when a timeout is pending
(timeouts are usually very rare, php processes will eventually be
restarted so this little memory waste won't have time to make any
impact.

Is this issue fixed in modern php ? If so I would be grateful for some
information about the way it was done. This would save me a lot of
time trying to trigger
a non existing confition.

I will try to reproduce this in a modern version of php.

It probably isn't. PG(last_error_message) is only modified in main.c, I would try to use a separate arena for PG(last_error_message) and PG(last_error_file). For instance it could be a static buffer reused during the whole execution and extended with mmap(2) in the unlikely case it turns out to be too small. I suspect it would also make the error handler faster, as it would avoid the free() + malloc()


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to