Missing null pointer check in archetype grow function.

[rj00a]

https://github.com/Ralith/hecs/blob/621c9f603b5d9a8847a7ef599947ceb3c2fbf8cd/src/archetype.rs#L281-L303

std::alloc::alloc may return a null pointer if memory allocation fails, but the code above doesn't check for this. This will result in undefined behavior.

As a side note, it would be more efficient to use std::alloc::realloc, so a fix should probably take this opportunity to make use of that.

[Ralith]

Thanks for the report! #348 should be a fix; a sanity-check on it would be welcome.

As a side note, it would be more efficient to use std::alloc::realloc

This would be difficult because an OOM might cause a panic while some, but not all, columns have been reallocated. We don't track this intermediate state in any way but through control flow, so there'd be no way for Archetype's Drop impl to know how large the allocation in need of freeing is. This could be mitigated with catch_unwind and a cursor, or something to that effect, but that's a lot of unsafe complexity for dubious benefit. Do any major allocators actually implement realloc?

[rj00a]

Do any major allocators actually implement realloc?

Rust's default System allocator calls the system's libc realloc, and every libc I looked at seems to implement realloc with some care.

Regardless, I agree that the complexity isn't really worth it. Lots of edge cases to worry about when stack unwinding is involved. One alternative is to store something like a BlobVec per column to track the allocations individually, but that's a fairly large change.

[Ralith]

Yeah, I don't see a case for it unless there's a significant benchmark impact or something, and this is all slowpath code anyway. Thanks for the review and discussion!