Skip to content

Naked calls to new and delete #222

@vinniefalco

Description

@vinniefalco

In the process of tracking down unbounded memory consumption in our application that uses rocksdb, a memory profiler pointed to ReadBlockContents as a large source of allocations (our version is a little older than what's in the master branch). I spent some time tracing the code and it looks like the local buf that is allocated via:
char* buf = new char[n + kBlockTrailerSize];
is later freed here:
https://github.com/facebook/rocksdb/blob/master/table/block.cc#L320

Studying the code more carefully it looks like a naked, allocated pointer is returned to the caller via Slice, and then ownership is regained by constructing a Block from BlockContents. The presumption is that result->heap_allocated becomes true in ReadBlockContents, causing the Block object to set owned_ to true, thus freeing the memory in Block::~Block.

We strongly discourage naked calls to new and delete as well as unowned naked pointers for the reason that it is hard to verify the chain of ownership of dynamically allocated objects. We are fairly certain there is a leak in there somewhere but analysis is impeded by the unsafe semantics. We suggest using the C++11 RAII containers such as std::unique_ptr in interfaces to control ownership.

Proposed change: replace bool BlockContents::heap_allocated with std::unique_ptr <char> BlockContents::allocation. Use the implicit conversion of unique_ptr to bool as a replacement for the value of heap_allocated. This way, ReadBlockContents can safely transfer ownership to the caller via the BlockContents::allocation field.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions