google / flatbuffers

FlatBuffers: Memory Efficient Serialization Library
https://flatbuffers.dev/
Apache License 2.0
22.56k stars 3.19k forks source link

ReleaseRaw cannot be used with custom allocators [C++] #8215

Open selcott opened 5 months ago

selcott commented 5 months ago

FlatBufferBuilder::ReleaseRaw() transfers ownership of the data without transferring ownership of or knowledge about the allocator that was used to allocate said data.

In order to deallocate memory returned from ReleaseRaw, the caller must:

  1. If the FlatBufferBuilder object was not initialized with a custom allocator
    1. Deallocate it with operator delete[]
  2. If the FlatBufferBuilder object was initialized with a custom allocator
    1. If own_allocator is false
      1. Must call the allocator (which you don’t have) ’s deallocate() function.
    2. If own_allocator is true
      1. Must make sure the FlatBufferBuilder object outlives your pointer (even though it was “released”) because otherwise:
        1. If the allocator owns all the memory that it dishes out and clears it all in its destructor:
          1. The FlatBufferBuilder object's destructor will delete the allocator, which will in turn deallocate the surrounding memory block while you’re still holding onto the pointer.
        2. If the allocator is just a tracking thing and gets its memory from the heap:
          1. Congratulations you didn’t crash, but best case scenario the allocator’s destructor would report a leak here at the very least, if the tracking it's doing is worth anything at all.
          2. And you have to know that that’s what this allocator does, in order to know that you still have to call operator delete[] later anyway.

The problem is not only that every call site of ReleaseRaw must follow the above logic, but rather that you cannot do so even if you wanted to, because the caller neither knows what allocator was used nor has any way of knowing the value of own_allocator.

Furthermore, even if those things were exposed, 2.ii.a, "Must make sure the FlatBufferBuilder object outlives your pointer," would still be a significant bug on its own, having "release" semantics that require the previous owner to outlive the new owner.