Don't clear metadata before transitioning chunk to owned state

[senderista]

While addressing last-minute review feedback, I got confused and forgot why I had used chunk_manager_t::load() instead of chunk_manager_t::initialize() in memory_manager_t::allocate_used_chunk(). The reason was that chunk_manager_t::initialize() clears chunk metadata, which we can't safely do until we know we own the chunk. This resulted in a race where a thread might initially try to acquire a used chunk and then get preempted, losing the race to acquire the chunk to another concurrent thread. Then the first thread would wake up and clear the now-in-use chunk's metadata while the second thread was already using it for allocations! This would clear any allocation bits that the second thread had already set, and the missing allocation bits would eventually trigger an assert when those allocations were freed during GC.

The fix was simple: just use chunk_manager_t::load() to access the metadata before acquiring the chunk, as I originally did. The metadata will be cleared anyway when gaia::db::allocate_object() (the original caller of memory_manager_t::allocate_chunk()) calls chunk_manager_t::initialize(), which calls chunk_manager_metadata_t::clear().

Note that the above initialization hazard doesn't apply to memory_manager_t::allocate_unused_chunk(), but there's no reason to do things any differently, so I made the same change there.

The fact that I got confused about initialization responsibility is evidence that this code needs further refactoring; the existing interfaces have to be contorted to fit the new chunk lifecycle protocol and should be replaced by a new interface design that starts from the protocol itself.

[LaurentiuCristofor]

I don't think the "NB" part is needed.

[LaurentiuCristofor]

BTW, I'm not sure if you have any valid use for initialize() anymore, so if that's the case, you should just remove it. The reason I asked you why you were not using it was because you still kept it, so I thought you still needed its semantics.

[senderista]

Yeah, I'm still calling it from gaia::db::allocate_object(), but as I said, this code is a mess and the control flow is quite convoluted. Really I just need to call chunk_manager_metadata_t::clear() somewhere, and only the chunk manager has access to chunk metadata, so that's why I still need it.