xcalloc called from signal handler

[Bernardo Innocenti]

Daniel Stone wrote:
> On Sun, Dec 16, 2007 at 06:48:09PM -0500, Bernardo Innocenti wrote:
>> Matthieu Herrb wrote:
>>> Unfortunalty the xserver 1.4 new Xinput code calls xcalloc() in
>>> functions that are called by the SIGIO handler. This has been identified
>>> as the cause of many X server segfaults on OpenBSD, and will probably
>>> also cause random problems on other systems.
>> I've observed such problems on Linux too, when you try to quit the X server
>> through SIGINT.
> 
> Feel free to file bugs (or, better, send patches) for malloc() on exit
> or in signal handlers.

This is my original post:
  http://lists.freedesktop.org/archives/xorg/2007-March/022406.html

And these are the bugs I filed back then:
  https://bugs.freedesktop.org/show_bug.cgi?id=10212
  https://bugs.freedesktop.org/show_bug.cgi?id=10213


The second one I never got to reproduce because I now have a working
XKB setup...


>> The error message is never going to show up because xcalloc() is
>> supposed to automatically abort() on failure.
> 
> Er, in which parallel universe?
> 
> _X_EXPORT void *
> Xcalloc(unsigned long amount)
> {
>     unsigned long   *ret;
> 
>     ret = Xalloc (amount);
>     if (ret)
>         bzero ((char *) ret, (int) amount);
>     return ret;
> }

Oh, I had assumed Xalloc() had this semantic because this is
what the xfoo() allocators do in GNU projects.

But, the real thing behaves like this:

_X_EXPORT void *
Xalloc(unsigned long amount)
{
    register pointer  ptr;

    if ((long)amount <= 0) {
        return (unsigned long *)NULL;
    }
    /* aligned extra on long word boundary */
    amount = (amount + (sizeof(long) - 1)) & ~(sizeof(long) - 1);
#ifdef MEMBUG
    if (!Must_have_memory && Memory_fail &&
        ((random() % MEM_FAIL_SCALE) < Memory_fail))
        return (unsigned long *)NULL;
#endif
    if ((ptr = (pointer)malloc(amount))) {
        return (unsigned long *)ptr;
    }
    if (Must_have_memory)
        FatalError("Out of memory");
    return (unsigned long *)NULL;
}

... the behavior is actually configurable through the global
variable Must_have_memory, which is usually NULL and gets
flicked to TRUE in specific places, like so:

        Must_have_memory = TRUE; /* XXX */
        spriteTrace = (WindowPtr *)xrealloc(
            spriteTrace, spriteTraceSize*sizeof(WindowPtr));
        Must_have_memory = FALSE; /* XXX */


What a horrible semantics for an allocator!

Would you approve a set of patches that converts our codebase to use
the regular malloc() and free() and kill off this custom allocator?
I'd be delighted to do it.

-- 
 \___/
 |___|   Bernardo Innocenti - http://www.codewiz.org/
  \___\  One Laptop Per Child - http://www.laptop.org/