realloc() on new thread incorrectly asserts on 3.5.0 on Linux

[Number5ix]

Running the following minimal program on Linux with MI_DEBUG=2 results in the program aborting. An internal mimalloc assertion can trip under some circumstances if realloc() is called on a brand new thread before before any other allocation happens.

#include <mimalloc.h>
#include <pthread.h>
#include <stdio.h>

#define BLOCK_SIZE 64
#define NEW_SIZE   40   // shrink, but stays >= BLOCK_SIZE/2: the "reuse the block" window

static void* worker(void* arg)
{
    // This thread has not allocated anything, so its thread-local theap is still the read-only
    // _mi_theap_empty, whose heap is NULL. Uncommenting either line below initializes it and
    // makes the abort go away:
    //
    //   void* warm = mi_malloc(8); mi_free(warm);
    //   mi_theap_t* theap = mi_theap_get_default();

    void* p = mi_realloc(arg, NEW_SIZE);
    printf("worker: realloc %d -> %d returned %p\n", BLOCK_SIZE, NEW_SIZE, p);
    return p;
}

int main(void)
{
    // allocated on the main thread, handed to a thread that has never allocated
    void* p = mi_malloc(BLOCK_SIZE);
    if (p == NULL) {
        printf("FAIL: initial allocation failed\n");
        return 1;
    }

    pthread_t t;
    if (pthread_create(&t, NULL, worker, p) != 0) {
        printf("FAIL: pthread_create\n");
        return 1;
    }

    void* result = NULL;
    pthread_join(t, &result);   // the abort happens before we get here

    if (result == NULL) {
        printf("FAIL: realloc returned NULL\n");
        return 1;
    }

    mi_free(result);
    printf("PASS\n");
    return 0;
}

e.g.

cc -O1 -g -DMI_DEBUG=2 -DMI_STATIC_LIB -Iinclude -Isrc realloc_uninit_theap.c src/static.c -o realloc_uninit_theap -lpthread -latomic
./realloc_uninit_theap

The issue appears to be in src/alloc.c, in mi_theap_realloc_zero_ex(), the "check if we can reuse the existing block" branch:

     if mi_unlikely(newsize<=size && newsize>=(size/2) && newsize>0) {
       #if MI_THEAP_INITASNULL
       if (theap!=NULL)
       #endif
       {
         if (mi_page_heap(page)==_mi_theap_heap(theap)) {   // <-- here

Under MI_TLS_MODEL_LOCAL only (why this is Linux-specific) a thread's __mi_theap_default starts out as &_mi_theap_empty -- non-NULL, but with a NULL heap -- and stays that way until the thread allocates for the first time.

_mi_theap_heap() asserts heap != NULL, so any realloc that goes down this branch on a thread that has not yet allocated trips the assertion.

Possible fix would be to change the test to mi_page_heap(page)==_mi_theap_heap_peek(theap) instead, as the heap will be initialized shortly after if it is NULL here.

The bug is sensitive to the requested reallocation size.

Conditions, measured against a 64-byte block (usable size 64):

newsize   worker allocated first?   result
-------   -----------------------   ------
40                no                abort      newsize <= size && newsize >= size/2
64                no                abort      upper bound of the reuse window
32                no                abort      lower bound of the reuse window
31                no                ok         below size/2: takes the general path
16                no                ok
200               no                ok         a grow: takes the general path
40                yes               ok         theap initialized, heap non-NULL

It needs a newsize that stays within the reuse window, on a thread whose first mimalloc call is the realloc itself. That combination is easy to hit with a worker/ingest thread that inherits a container from another thread and compacts it -- which is how we found it, via a hashtable shrink on a network poll thread.

[daanx]

Thank you! I applied the suggested fix as that seems the right approach.