facebook / facebook/folly

Question about using folly::IOBuf::takeOwnerShip() with flatbuffers.

Open
#1,499 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

I am using proxygen as a webserver for my C++ application and using flatbuffers to encode the response body. After creating the response buffer, I need to transfer ownership to folly::IOBuf to avoid the copy and avoid keeping a member variable in my handler class to extend the lifetime of the `flatbuffers::FlatbufferBuilder` object. So I came up with this:

```cpp
auto xyz_vector = get_xyz_vector();
auto xyz_response_buffer = CreateXYZResponse(
xyz_response_builder, xyz_response_builder.CreateVectorOfStrings(xyz_vector));
xyz_response_builder.Finish(xyz_response_buffer);
size_t allocated_bytes = 0;
size_t buffer_offset = 0;
const auto buffer_ptr = xyz_response_builder.ReleaseRaw(allocated_bytes, buffer_offset);
struct BufferInfo
{
size_t buffer_offset = 0;
size_t allocated_bytes = 0;
BufferInfo(size_t offset, size_t capacity) : buffer_offset(offset), allocated_bytes(capacity)
{}
};
auto buffer_info = new BufferInfo(buffer_offset, allocated_bytes);
using BufferPtrType = decltype(buffer_ptr);
return folly::IOBuf::takeOwnership(
buffer_ptr + buffer_offset, // This is passing in the start of flatbuffer instead of the actual buffer.
allocated_bytes,
allocated_bytes - buffer_offset,
[](void* buffer_ptr_with_offset, void* buffer_info_ptr) {
const auto buffer_info = reinterpret_cast(buffer_info_ptr);
flatbuffers::DefaultAllocator::dealloc(
// Here we adjust the pointer to point to the start of the actual buffer.
reinterpret_cast(buffer_ptr_with_offset) - buffer_info->buffer_offset,
buffer_info->allocated_bytes);
delete buffer_info;
},
buffer_info);
```

As noted in the comments, here I am lying to `takeOwnerShip()` function about the start of the buffer because otherwise I wont be able to send the correct flatbuffer in the response body and the client will fail to deserialize it. In the deallocator, I again adjust the buffer pointer to point to the actual buffer so that we can correctly deallocate it. I tested this with an ASAN build (using clang 10) and it seems to work.
Is this approach correct? In particular, is passing in a "shifted" buffer pointer to `takeOwnerShip` guaranteed to work. Does it break any assumptions that `folly::IOBuf` class makes about the buffer pointer. Please let me know if you need more details.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.