Adar Dembo has submitted this change and it was merged.

Change subject: KUDU-1931: fix SIGSEGV in file cache 
RandomAccessFile::memory_footprint()
......................................................................


KUDU-1931: fix SIGSEGV in file cache RandomAccessFile::memory_footprint()

TIL that the pointer returned by std::make_shared().get() or
*std::make_shared() isn't the base of the underlying heap memory allocation,
because the shared_ptr control block _precedes_ the object pointer.

This means that kudu_malloc_usable_size(this) is totally unsafe if 'this'
was allocated via std::make_shared(). With glibc's malloc, it caused a
crash. Thankfully tcmalloc is more forgiving: it finds its allocations'
bookkeeping information by converting addresses into their underlying pages,
so a crash would only occur if there was a page boundary between the
shared_ptr control block and the object's pointer address (I don't even know
if this is possible).

I found this bug by accident when I ran an in-progress unit test with
--block_manager=file. This speaks to our lack of FBM coverage. For now, I've
added some coverage but we really should be running at least one end-to-end
test on all block managers; I filed KUDU-1932 to track that.

Change-Id: I9e57c06dd35252d03729c10ff64fe43d79288513
Reviewed-on: http://gerrit.cloudera.org:8080/6359
Reviewed-by: David Ribeiro Alves <[email protected]>
Tested-by: Kudu Jenkins
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/util/file_cache-test.cc
M src/kudu/util/file_cache.cc
3 files changed, 38 insertions(+), 1 deletion(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6359
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9e57c06dd35252d03729c10ff64fe43d79288513
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>

Reply via email to