2013/9/18 Ángel González <keis...@gmail.com>: > 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.
time() refers to https://bugs.php.net/bug.php?id=31749, You are right this deadlock is related to free() >> 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() thank You I didn't notice that it is used only there, i will try to use a static buffer. I managed to produce a segfault on current php version (php heap corruption), bug report https://bugs.php.net/bug.php?id=65674 spprintf() allocating memory is also not safe. I will try to fix this by using a static buffer. Thanks, Michal Grzedzicki -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
