search

microsoft / mimalloc

mimalloc is a compact general purpose allocator with excellent performance.
MIT License
9.74k stars 791 forks source link

Data race with `mi_page_s`'s `flags` field #865

Closed Zoxc closed 3 months ago

Zoxc commented 3 months ago

In _mi_free_generic when freeing a non-local allocation, the flags field of the page is accessed without synchronization. This leads to a data race when the thread owning the page also modifies the flags field.

daanx commented 3 months ago

Ah, that is right! I remember considering the race benign "enough" when writing this code but now that I look at it again I think it should be fixed -- let me look into it.

Why it is relatively benign is that the "aligned" field of the flags is monotonically increasing -- and if it increases it doesn't matter for the current pointer that is freed. As long as a race access doesn't return some completely arbitrary value it is fine (and thus ok on most/all hardware). However, it is of course not within C semantics or guarantees and we should fix this. Thanks!

daanx commented 3 months ago

Thanks @zoxc for the feedback; I refactored the free-ing code in a separate file and removed the "benign" race; and also improved the ptr_unalign to be faster as it is now always used for a non-local free (by adding a page_start and block_size_shift field).