From f8999fcf31be758d99fac3f64bc4ca0717b7f576 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Sun, 21 Dec 2014 00:23:28 -0800 Subject: [PATCH] Fix a SIGSEGV in BackgroundFlush Summary: This one wasn't easy to find :) What happens is we go through all cfds on flush_queue_ and find no cfds to flush, *but* the cfd is set to the last CF we looped through and following code assumes we want it flushed. BTW @sdong do you think we should also make BackgroundFlush() only check a single cfd for flushing instead of doing this `while (!flush_queue_.empty())`? Test Plan: regression test no longer fails Reviewers: sdong, rven, yhchiang Reviewed By: yhchiang Subscribers: sdong, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D30591 --- db/db_impl.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index cb5dcc59c..aff68ed45 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1801,17 +1801,18 @@ Status DBImpl::BackgroundFlush(bool* madeProgress, JobContext* job_context, ColumnFamilyData* cfd = nullptr; while (!flush_queue_.empty()) { // This cfd is already referenced - cfd = PopFirstFromFlushQueue(); + auto first_cfd = PopFirstFromFlushQueue(); - if (cfd->IsDropped() || !cfd->imm()->IsFlushPending()) { + if (first_cfd->IsDropped() || !first_cfd->imm()->IsFlushPending()) { // can't flush this CF, try next one - if (cfd->Unref()) { - delete cfd; + if (first_cfd->Unref()) { + delete first_cfd; } continue; } // found a flush! + cfd = first_cfd; break; }