From 41cb922b34fd231bfb0e78af92168375b57b308f Mon Sep 17 00:00:00 2001 From: Abhishek Kona Date: Mon, 29 Apr 2013 13:19:24 -0700 Subject: [PATCH] Allocate the LogReporter from heap. Summary: Summary: The current code has a bug that take address of stack allocated LogReporter. It is causing SIGSEGV because the stack address is no longer valid when referenced. Test Plan: Tested on prod. Reviewers: haobo, dhruba, heyongqiang Reviewed By: heyongqiang Differential Revision: https://reviews.facebook.net/D10557 --- db/transaction_log_iterator_impl.cc | 18 +++++------------- db/transaction_log_iterator_impl.h | 6 ++---- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/db/transaction_log_iterator_impl.cc b/db/transaction_log_iterator_impl.cc index 1b0f25b2e..0b1f47d72 100644 --- a/db/transaction_log_iterator_impl.cc +++ b/db/transaction_log_iterator_impl.cc @@ -22,15 +22,9 @@ TransactionLogIteratorImpl::TransactionLogIteratorImpl( lastFlushedSequence_(lastFlushedSequence) { assert(files_.get() != nullptr); assert(lastFlushedSequence_); -} -LogReporter -TransactionLogIteratorImpl::NewLogReporter(const uint64_t logNumber) { - LogReporter reporter; - reporter.env = options_->env; - reporter.info_log = options_->info_log.get(); - reporter.log_number = logNumber; - return reporter; + reporter_.env = options_->env; + reporter_.info_log = options_->info_log.get(); } Status TransactionLogIteratorImpl::OpenLogFile( @@ -74,7 +68,6 @@ bool TransactionLogIteratorImpl::Valid() { void TransactionLogIteratorImpl::Next() { LogFile currentLogFile = files_.get()->at(currentFileIndex_); - LogReporter reporter = NewLogReporter(currentLogFile.logNumber); // First seek to the given seqNo. in the current file. std::string scratch; @@ -95,7 +88,7 @@ void TransactionLogIteratorImpl::Next() { } while (currentLogReader_->ReadRecord(&record, &scratch)) { if (record.size() < 12) { - reporter.Corruption( + reporter_.Corruption( record.size(), Status::Corruption("log record too small")); continue; } @@ -122,7 +115,7 @@ void TransactionLogIteratorImpl::Next() { } while (currentLogReader_->ReadRecord(&record, &scratch)) { if (record.size() < 12) { - reporter.Corruption( + reporter_.Corruption( record.size(), Status::Corruption("log record too small")); continue; } else { @@ -165,7 +158,6 @@ void TransactionLogIteratorImpl::UpdateCurrentWriteBatch(const Slice& record) { } Status TransactionLogIteratorImpl::OpenLogReader(const LogFile& logFile) { - LogReporter reporter = NewLogReporter(logFile.logNumber); unique_ptr file; Status status = OpenLogFile(logFile, &file); if (!status.ok()) { @@ -173,7 +165,7 @@ Status TransactionLogIteratorImpl::OpenLogReader(const LogFile& logFile) { } assert(file); currentLogReader_.reset( - new log::Reader(std::move(file), &reporter, true, 0) + new log::Reader(std::move(file), &reporter_, true, 0) ); return Status::OK(); } diff --git a/db/transaction_log_iterator_impl.h b/db/transaction_log_iterator_impl.h index f682fabc7..90fd1fcf0 100644 --- a/db/transaction_log_iterator_impl.h +++ b/db/transaction_log_iterator_impl.h @@ -17,10 +17,8 @@ namespace leveldb { struct LogReporter : public log::Reader::Reporter { Env* env; Logger* info_log; - uint64_t log_number; virtual void Corruption(size_t bytes, const Status& s) { - Log(info_log, "%ld: dropping %d bytes; %s", - log_number, static_cast(bytes), s.ToString().c_str()); + Log(info_log, "dropping %zu bytes; %s", bytes, s.ToString().c_str()); } }; @@ -54,7 +52,7 @@ class TransactionLogIteratorImpl : public TransactionLogIterator { std::unique_ptr currentBatch_; unique_ptr currentLogReader_; Status OpenLogFile(const LogFile& logFile, unique_ptr* file); - LogReporter NewLogReporter(uint64_t logNumber); + LogReporter reporter_; SequenceNumber const * const lastFlushedSequence_; // represents the sequence number being read currently. SequenceNumber currentSequence_;