]> granicus.if.org Git - esp-idf/commitdiff
heap: Fix race condition causing malloc() to fail under some conditions
authorAngus Gratton <angus@espressif.com>
Thu, 19 Oct 2017 07:59:04 +0000 (15:59 +0800)
committerAngus Gratton <gus@projectgus.com>
Thu, 19 Oct 2017 08:05:00 +0000 (16:05 +0800)
During a call to multi_heap_malloc(), if both these conditions were true:
- That heap only has one block large enough for the allocation
  (this is always the case if the heap is unfragmented).
- Another allocation is simultaneously occurring in the same heap.

... multi_heap_malloc() could incorrectly return NULL.

This caused IDF heap_caps_malloc() and malloc() to also fail, particularly
often if only one or two heaps had space for the allocation (otherwise
heap_caps_malloc() fails over to the next heap).

components/heap/multi_heap.c

index b0452f4c4e2abc3e930ce45d8c08bb9bc3281a27..cc621c0d88b13a580e8a1eaa78e13dc8fe19dc6b 100644 (file)
@@ -337,12 +337,22 @@ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size)
     size_t best_size = SIZE_MAX;
     size = ALIGN_UP(size);
 
-    if (size == 0 || heap == NULL || heap->free_bytes < size) {
+    if (size == 0 || heap == NULL) {
         return NULL;
     }
 
     MULTI_HEAP_LOCK(heap->lock);
 
+    /* Note: this check must be done while holding the lock as both
+       malloc & realloc may temporarily shrink the free_bytes value
+       before they split a large block. This can result in false negatives,
+       especially if the heap is unfragmented.
+    */
+    if (heap->free_bytes < size) {
+        MULTI_HEAP_UNLOCK(heap->lock);
+        return NULL;
+    }
+
     /* Find best free block to perform the allocation in */
     prev = &heap->first_block;
     for (heap_block_t *b = heap->first_block.next_free; b != NULL; b = b->next_free) {