Re: free deadlock in timeout signal handler
2013/9/18 Ángel González <[email protected]>:
> 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
Thread (3 messages)