Commit bee49a23 authored by bcwhite's avatar bcwhite Committed by Commit bot

Clear memory in a predictable and atomic manner when changing type.

There may be existing pointers to the memory block that are being actively
accessed while the zeroing of memory is taking place. While this change
doesn't fix that, it does make it possible for those other threads to recognize
that memory is changing under it and be able to react appropriately.

Also, restore missing kTypeIdAny that was somehow lost.

BUG=620813

Review-Url: https://codereview.chromium.org/2709113003
Cr-Commit-Position: refs/heads/master@{#456121}
parent 9558316b
......@@ -540,10 +540,19 @@ bool PersistentMemoryAllocator::ChangeType(Reference ref,
return false;
}
// Clear the memory while the type doesn't match either "from" or "to".
memset(const_cast<char*>(reinterpret_cast<volatile char*>(block) +
sizeof(BlockHeader)),
0, block->size - sizeof(BlockHeader));
// Clear the memory in an atomic manner. Using "release" stores force
// every write to be done after the ones before it. This is better than
// using memset because (a) it supports "volatile" and (b) it creates a
// reliable pattern upon which other threads may rely.
volatile std::atomic<int>* data =
reinterpret_cast<volatile std::atomic<int>*>(
reinterpret_cast<volatile char*>(block) + sizeof(BlockHeader));
const uint32_t words = (block->size - sizeof(BlockHeader)) / sizeof(int);
DCHECK_EQ(0U, (block->size - sizeof(BlockHeader)) % sizeof(int));
for (uint32_t i = 0; i < words; ++i) {
data->store(0, std::memory_order_release);
++data;
}
// If the destination type is "transitioning" then skip the final exchange.
if (to_type_id == kTypeIdTransitioning)
......
......@@ -217,6 +217,9 @@ class BASE_EXPORT PersistentMemoryAllocator {
};
enum : uint32_t {
// A value that will match any type when doing lookups.
kTypeIdAny = 0x00000000,
// A value indicating that the type is in transition. Work is being done
// on the contents to prepare it for a new type to come.
kTypeIdTransitioning = 0xFFFFFFFF,
......@@ -400,7 +403,8 @@ class BASE_EXPORT PersistentMemoryAllocator {
// Changing the type doesn't mean the data is compatible with the new type.
// Passing true for |clear| will zero the memory after the type has been
// changed away from |from_type_id| but before it becomes |to_type_id| meaning
// that it is done in a manner that is thread-safe.
// that it is done in a manner that is thread-safe. Memory is guaranteed to
// be zeroed atomically by machine-word in a monotonically increasing order.
//
// It will likely be necessary to reconstruct the type before it can be used.
// Changing the type WILL NOT invalidate existing pointers to the data, either
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment