Skip to content

Commit

Permalink
Fix: improve storage tool checks, disable default allocator check, fi…
Browse files Browse the repository at this point in the history 
…x mwcsys_executil (#289)

* Fix: safer memory management in the storage tool

* Fix: pass default allocators to constructed objects

* Suppress AIX/Solaris default allocator check in storage tool UT

Signed-off-by: Evgeny Malygin <[email protected]>
  • Loading branch information
@678098
678098 authored May 17, 2024
1 parent 544a3c0 commit ef0508c
Show file tree
Hide file tree
Showing 16 changed files with 181 additions and 134 deletions.
19 changes: 12 additions & 7 deletions src/applications/bmqstoragetool/m_bmqstoragetool_commandprocessorfactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,25 @@ CommandProcessorFactory::createCommandProcessor(
bsl::ostream& ostream,
bslma::Allocator* allocator)
{
// PRECONDITIONS
BSLS_ASSERT(params);

bslma::Allocator* alloc = bslma::Default::allocator(allocator);

// Create searchResult for given 'params'.
bsl::shared_ptr<SearchResult> searchResult =
SearchResultFactory::createSearchResult(params,
fileManager,
ostream,
allocator);
alloc);
// Create commandProcessor.
bslma::ManagedPtr<CommandProcessor> commandProcessor(
new (*allocator) JournalFileProcessor(params,
fileManager,
searchResult,
ostream,
allocator),
allocator);
new (*alloc) JournalFileProcessor(params,
fileManager,
searchResult,
ostream,
alloc),
alloc);
return commandProcessor;
}

Expand Down
2 changes: 1 addition & 1 deletion src/applications/bmqstoragetool/m_bmqstoragetool_commandprocessorfactory.t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static void test1_breathingTest()
{
mwctst::TestHelper::printTestName("BREATHING TEST");
// Empty parameters
CommandLineArguments arguments;
CommandLineArguments arguments(s_allocator_p);
Parameters params(arguments, s_allocator_p);
bslma::ManagedPtr<FileManager> fileManager(new (*s_allocator_p)
FileManagerMock(),
Expand Down
20 changes: 11 additions & 9 deletions src/applications/bmqstoragetool/m_bmqstoragetool_filemanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,29 +95,31 @@ mqbs::DataFileIterator* FileManagerImpl::dataFileIterator()
QueueMap FileManagerImpl::buildQueueMap(const bsl::string& cslFile,
bslma::Allocator* allocator)
{
bslma::Allocator* alloc = bslma::Default::allocator(allocator);

using namespace bmqp_ctrlmsg;
typedef bsl::vector<QueueInfo> QueueInfos;
typedef QueueInfos::const_iterator QueueInfosIt;
if (cslFile.empty()) {
throw bsl::runtime_error("empty CSL file path"); // THROW
}
QueueMap d_queueMap(allocator);
QueueMap d_queueMap(alloc);

// Required for ledger operations
bmqp::Crc32c::initialize();

// Instantiate ledger config
mqbsi::LedgerConfig ledgerConfig(allocator);
mqbsi::LedgerConfig ledgerConfig(alloc);
bsl::shared_ptr<mqbsi::LogIdGenerator> logIdGenerator(
new (*allocator) mqbmock::LogIdGenerator("bmq_csl_", allocator),
allocator);
new (*alloc) mqbmock::LogIdGenerator("bmq_csl_", alloc),
alloc);
bsl::shared_ptr<mqbsi::LogFactory> logFactory(
new (*allocator) mqbsl::MemoryMappedOnDiskLogFactory(allocator),
allocator);
new (*alloc) mqbsl::MemoryMappedOnDiskLogFactory(alloc),
alloc);
bdls::FilesystemUtil::Offset fileSize = bdls::FilesystemUtil::getFileSize(
cslFile.c_str());
bsl::string pattern(allocator);
bsl::string location(allocator);
bsl::string pattern(alloc);
bsl::string location(alloc);
BSLS_ASSERT(bdls::PathUtil::getBasename(&pattern, cslFile) == 0);
BSLS_ASSERT(bdls::PathUtil::getDirname(&location, cslFile) == 0);
ledgerConfig.setLocation(location)
Expand All @@ -133,7 +135,7 @@ QueueMap FileManagerImpl::buildQueueMap(const bsl::string& cslFile,
.setValidateLogCallback(mqbc::ClusterStateLedgerUtil::validateLog);

// Create and open the ledger
mqbsl::Ledger ledger(ledgerConfig, allocator);
mqbsl::Ledger ledger(ledgerConfig, alloc);
BSLS_ASSERT(ledger.open(mqbsi::Ledger::e_READ_ONLY) == 0);
// Set guard to close the ledger
bdlb::ScopeExitAny guard(bdlf::BindUtil::bind(closeLedger, &ledger));
Expand Down
1 change: 1 addition & 0 deletions src/applications/bmqstoragetool/m_bmqstoragetool_filemanager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class FileManagerImpl : public FileManager {
// PRIVATE DATA
FileHandler<mqbs::JournalFileIterator> d_journalFile;
// Handler of journal file

FileHandler<mqbs::DataFileIterator> d_dataFile;
// Handler of data file

Expand Down
3 changes: 3 additions & 0 deletions src/applications/bmqstoragetool/m_bmqstoragetool_journalfileprocessor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ namespace m_bmqstoragetool {
int moveToLowerBound(mqbs::JournalFileIterator* jit,
const bsls::Types::Uint64& timestamp)
{
// PRECONDITIONS
BSLS_ASSERT(jit);

int rc = 1;
const unsigned int recordSize = jit->header().recordWords() *
bmqp::Protocol::k_WORD_SIZE;
Expand Down
6 changes: 6 additions & 0 deletions src/applications/bmqstoragetool/m_bmqstoragetool_journalfileprocessor.t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,12 @@ static void test12_printMessagesDetailsTest()
{
mwctst::TestHelper::printTestName("PRINT MESSAGE DETAILS TEST");

#if defined(BSLS_PLATFORM_OS_SOLARIS) || defined(BSLS_PLATFORM_OS_AIX)
s_ignoreCheckDefAlloc = true;
// Disable default allocator check for this test until we can debug
// it on AIX/Solaris
#endif

// Simulate journal file
const size_t k_NUM_RECORDS = 15;
RecordsListType records(s_allocator_p);
Expand Down
3 changes: 3 additions & 0 deletions src/applications/bmqstoragetool/m_bmqstoragetool_messagedetails.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ bool findQueueAppIdByAppKey(
const bsl::vector<BloombergLP::bmqp_ctrlmsg::AppIdInfo>& appIds,
const mqbu::StorageKey& appKey)
{
// PRECONDITIONS
BSLS_ASSERT(appId);

if (appKey.isNull())
return false; // RETURN

Expand Down
2 changes: 1 addition & 1 deletion src/applications/bmqstoragetool/m_bmqstoragetool_parameters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ bool CommandLineArguments::validate(bsl::string* error)
mwcu::MemOutStream ss;

if (d_journalPath.empty() && d_journalFile.empty()) {
ss << "Niether journal path nor journal file are specified\n";
ss << "Neither journal path nor journal file are specified\n";
}
else if (!d_journalPath.empty()) {
if (d_journalFile.empty() && d_dataFile.empty()) {
Expand Down
10 changes: 8 additions & 2 deletions src/applications/bmqstoragetool/m_bmqstoragetool_queuemap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,27 @@ void QueueMap::update(const bmqp_ctrlmsg::QueueInfoUpdate& queueInfoUpdate)
bool QueueMap::findInfoByKey(bmqp_ctrlmsg::QueueInfo* queueInfo_p,
const mqbu::StorageKey& key) const
{
// PRECONDITIONS
BSLS_ASSERT(queueInfo_p);

QueueKeyToInfoMap::const_iterator it = d_queueKeyToInfoMap.find(key);
if (it != d_queueKeyToInfoMap.end()) {
*queueInfo_p = it->second;
return true;
return true; // RETURN
}
return false;
}

bool QueueMap::findKeyByUri(mqbu::StorageKey* queueKey_p,
const bsl::string& uri) const
{
// PRECONDITIONS
BSLS_ASSERT(queueKey_p);

QueueUriToKeyMap::const_iterator it = d_queueUriToKeyMap.find(uri);
if (it != d_queueUriToKeyMap.end()) {
*queueKey_p = it->second;
return true;
return true; // RETURN
}
return false;
}
Expand Down
25 changes: 14 additions & 11 deletions src/applications/bmqstoragetool/m_bmqstoragetool_searchresult.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void printDataFileMeta(bsl::ostream& ostream,
mqbs::DataFileIterator* dataFile_p)
{
if (!dataFile_p || !dataFile_p->isValid()) {
return;
return; // RETURN
}
ostream << "\nDetails of data file: \n"
<< *dataFile_p->mappedFileDescriptor() << " "
Expand All @@ -50,7 +50,7 @@ void printJournalFileMeta(bsl::ostream& ostream,
bslma::Allocator* allocator)
{
if (!journalFile_p || !journalFile_p->isValid()) {
return;
return; // RETURN
}

ostream << "\nDetails of journal file: \n";
Expand Down Expand Up @@ -124,7 +124,8 @@ void printJournalFileMeta(bsl::ostream& ostream,

bsls::Types::Uint64 epochValue = syncPt.header().timestamp();
bdlt::Datetime datetime;
int rc = bdlt::EpochUtil::convertFromTimeT64(&datetime, epochValue);
const int rc = bdlt::EpochUtil::convertFromTimeT64(&datetime,
epochValue);
if (0 != rc) {
printer << 0;
}
Expand Down Expand Up @@ -289,10 +290,10 @@ bool SearchResultTimestampDecorator::processDeletionRecord(
SearchShortResult::SearchShortResult(
bsl::ostream& ostream,
bslma::ManagedPtr<PayloadDumper>& payloadDumper,
bslma::Allocator* allocator,
const bool printImmediately,
const bool eraseDeleted,
const bool printOnDelete)
bool printImmediately,
bool eraseDeleted,
bool printOnDelete,
bslma::Allocator* allocator)
: d_ostream(ostream)
, d_payloadDumper(payloadDumper)
, d_printImmediately(printImmediately)
Expand Down Expand Up @@ -408,10 +409,10 @@ SearchDetailResult::SearchDetailResult(
bsl::ostream& ostream,
const QueueMap& queueMap,
bslma::ManagedPtr<PayloadDumper>& payloadDumper,
bslma::Allocator* allocator,
const bool printImmediately,
const bool eraseDeleted,
const bool cleanUnprinted)
bool printImmediately,
bool eraseDeleted,
bool cleanUnprinted,
bslma::Allocator* allocator)
: d_ostream(ostream)
, d_queueMap(queueMap)
, d_payloadDumper(payloadDumper)
Expand Down Expand Up @@ -540,6 +541,7 @@ bool SearchDetailResult::hasCache() const
// ========================
// class SearchAllDecorator
// ========================

SearchAllDecorator::SearchAllDecorator(
const bsl::shared_ptr<SearchResult>& component,
bslma::Allocator* allocator)
Expand Down Expand Up @@ -617,6 +619,7 @@ void SearchOutstandingDecorator::outputResult()
// =======================================
// class SearchPartiallyConfirmedDecorator
// =======================================

SearchPartiallyConfirmedDecorator::SearchPartiallyConfirmedDecorator(
const bsl::shared_ptr<SearchResult>& component,
bsl::ostream& ostream,
Expand Down
22 changes: 11 additions & 11 deletions src/applications/bmqstoragetool/m_bmqstoragetool_searchresult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ class SearchShortResult : public SearchResult {
public:
// CREATORS

/// Constructor using the specified `ostream`, `payloadDumper`, `allocator`
/// `printImmediately`, `eraseDeleted`, and `printOnDelete`.
/// Constructor using the specified `ostream`, `payloadDumper`,
/// `printImmediately`, `eraseDeleted`, `printOnDelete` and `allocator`.
explicit SearchShortResult(bsl::ostream& ostream,
bslma::ManagedPtr<PayloadDumper>& payloadDumper,
bslma::Allocator* allocator,
const bool printImmediately = true,
const bool eraseDeleted = false,
const bool printOnDelete = false);
bool printImmediately = true,
bool eraseDeleted = false,
bool printOnDelete = false,
bslma::Allocator* allocator = 0);

// MANIPULATORS

Expand Down Expand Up @@ -274,14 +274,14 @@ class SearchDetailResult : public SearchResult {
// CREATORS

/// Constructor using the specified `ostream`, `queueMap`, `payloadDumper`,
/// `allocator` `printImmediately`, `eraseDeleted`, and `cleanUnprinted`.
/// `printImmediately`, `eraseDeleted`, `cleanUnprinted` and `allocator`.
SearchDetailResult(bsl::ostream& ostream,
const QueueMap& queueMap,
bslma::ManagedPtr<PayloadDumper>& payloadDumper,
bslma::Allocator* allocator,
const bool printImmediately = true,
const bool eraseDeleted = true,
const bool cleanUnprinted = false);
bool printImmediately = true,
bool eraseDeleted = true,
bool cleanUnprinted = false,
bslma::Allocator* allocator = 0);

// MANIPULATORS

Expand Down
Loading

0 comments on commit ef0508c

Please sign in to comment.