Skip to content

Commit

Permalink
Fix memory access violations, replace boost::format
Browse files Browse the repository at this point in the history 
- Fix memory access violations related to the persistence path
- Replace boost::format with strprintf in mastercore.cpp
- Replace boost::format with strprintf in mastercore_sp.h
- Replace boost::format with strprintf in mastercore_sp.cp
- Minor, low hanging changes on the way

The replacement of boost::format was done, because tinyformat has similar properties, and is used in many other places of the project anyway. On top, tinyformat should be faster, but I didn't confirm this.

The combination of boost::filesystem::path, boost::format, string(), str() and c_str() caused memory related issues, according to valgrind.

Before:
- https://gist.githubusercontent.com/dexX7/d1298afb7ded4a50a129/raw/

After:
- https://gist.githubusercontent.com/dexX7/8a78b4e84a4abc3ecb24/raw/
  • Loading branch information
@dexX7
dexX7 committed Apr 28, 2015
1 parent af1d7b4 commit 0e397bd
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 75 deletions.
60 changes: 30 additions & 30 deletions src/mastercore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#include <boost/exception/to_string.hpp>
#include <boost/filesystem.hpp>
#include <boost/foreach.hpp>
#include <boost/format.hpp>
#include <boost/lexical_cast.hpp>
#include <boost/multiprecision/cpp_int.hpp>

Expand Down Expand Up @@ -355,7 +354,7 @@ CMPPending pending;
}

// this is the master list of all amounts for all addresses for all properties, map is sorted by Bitcoin address
std::map<string, CMPTally> mastercore::mp_tally_map;
std::map<std::string, CMPTally> mastercore::mp_tally_map;

CMPTally *mastercore::getTally(const string & address)
{
Expand Down Expand Up @@ -2040,8 +2039,9 @@ static int load_most_relevant_state()
if (persistedBlocks.find(spBlockIndex->GetBlockHash()) != persistedBlocks.end()) {
int success = -1;
for (int i = 0; i < NUM_FILETYPES; ++i) {
const string filename = (MPPersistencePath / (boost::format("%s-%s.dat") % statePrefix[i] % curTip->GetBlockHash().ToString()).str().c_str()).string();
success = msc_file_load(filename, i, true);
boost::filesystem::path path = MPPersistencePath / strprintf("%s-%s.dat", statePrefix[i], curTip->GetBlockHash().ToString());
const std::string strFile = path.string();
success = msc_file_load(strFile, i, true);
if (success < 0) {
break;
}
Expand Down Expand Up @@ -2103,13 +2103,12 @@ static int write_msc_balances(ofstream &file, SHA256_CTX *shaCtx)

emptyWallet = false;

lineOut.append((boost::format("%d:%d,%d,%d,%d;")
% prop
% balance
% sellReserved
% acceptReserved
% metadexReserve).str());

lineOut.append(strprintf("%d:%d,%d,%d,%d;",
prop,
balance,
sellReserved,
acceptReserved,
metadexReserve));
}

if (false == emptyWallet) {
Expand Down Expand Up @@ -2176,11 +2175,10 @@ static int write_globals_state(ofstream &file, SHA256_CTX *shaCtx)
{
unsigned int nextSPID = _my_sps->peekNextSPID(OMNI_PROPERTY_MSC);
unsigned int nextTestSPID = _my_sps->peekNextSPID(OMNI_PROPERTY_TMSC);
string lineOut = (boost::format("%d,%d,%d")
% exodus_prev
% nextSPID
% nextTestSPID
).str();
std::string lineOut = strprintf("%d,%d,%d",
exodus_prev,
nextSPID,
nextTestSPID);

// add the line to the hash
SHA256_Update(shaCtx, lineOut.c_str(), lineOut.length());
Expand All @@ -2205,10 +2203,11 @@ static int write_mp_crowdsales(ofstream &file, SHA256_CTX *shaCtx)

static int write_state_file( CBlockIndex const *pBlockIndex, int what )
{
const char *blockHash = pBlockIndex->GetBlockHash().ToString().c_str();
boost::filesystem::path balancePath = MPPersistencePath / (boost::format("%s-%s.dat") % statePrefix[what] % blockHash).str();
ofstream file;
file.open(balancePath.string().c_str());
boost::filesystem::path path = MPPersistencePath / strprintf("%s-%s.dat", statePrefix[what], pBlockIndex->GetBlockHash().ToString());
const std::string strFile = path.string();

std::ofstream file;
file.open(strFile.c_str());

SHA256_CTX shaCtx;
SHA256_Init(&shaCtx);
Expand Down Expand Up @@ -2307,9 +2306,10 @@ static void prune_state_files( CBlockIndex const *topIndex )
}

// destroy the associated files!
const char *blockHashStr = (*iter).ToString().c_str();
std::string strBlockHash = iter->ToString();
for (int i = 0; i < NUM_FILETYPES; ++i) {
boost::filesystem::remove(MPPersistencePath / (boost::format("%s-%s.dat") % statePrefix[i] % blockHashStr).str());
boost::filesystem::path path = MPPersistencePath / strprintf("%s-%s.dat", statePrefix[i], strBlockHash);
boost::filesystem::remove(path);
}
}
}
Expand Down Expand Up @@ -2419,7 +2419,7 @@ int mastercore_init()
_my_sps = new CMPSPInfo(GetDataDir() / "MP_spinfo", false);

MPPersistencePath = GetDataDir() / "MP_persist";
boost::filesystem::create_directories(MPPersistencePath);
TryCreateDirectory(MPPersistencePath);

// legacy code, setting to pre-genesis-block
static int snapshotHeight = (GENESIS_BLOCK - 1);
Expand Down Expand Up @@ -4509,13 +4509,13 @@ int rc = PKT_ERROR_STO -1000;

totalTokens = 0;

typedef std::set<pair<int64_t, string>, SendToOwners_compare> OwnerAddrType;
typedef std::set<std::pair<int64_t, std::string>, SendToOwners_compare> OwnerAddrType;
OwnerAddrType OwnerAddrSet;

{
for(map<string, CMPTally>::reverse_iterator my_it = mp_tally_map.rbegin(); my_it != mp_tally_map.rend(); ++my_it)
for (std::map<std::string, CMPTally>::reverse_iterator my_it = mp_tally_map.rbegin(); my_it != mp_tally_map.rend(); ++my_it)
{
const string address = (my_it->first).c_str();
const std::string address = my_it->first;

// do not count the sender
if (address == sender) continue;
Expand All @@ -4529,7 +4529,7 @@ int rc = PKT_ERROR_STO -1000;

if (tokens)
{
OwnerAddrSet.insert(make_pair(tokens, address));
OwnerAddrSet.insert(std::make_pair(tokens, address));
totalTokens += tokens;
}
}
Expand All @@ -4547,11 +4547,11 @@ int rc = PKT_ERROR_STO -1000;
{ // two-iteration loop START
// split up what was taken and distribute between all holders
uint64_t sent_so_far = 0;
for(OwnerAddrType::reverse_iterator my_it = OwnerAddrSet.rbegin(); my_it != OwnerAddrSet.rend(); ++my_it)
for (OwnerAddrType::reverse_iterator my_it = OwnerAddrSet.rbegin(); my_it != OwnerAddrSet.rend(); ++my_it)
{ // owners loop
const string address = my_it->second;
const std::string address = my_it->second;

int128_t owns = my_it->first;
int128_t owns = int128_t(my_it->first);
int128_t temp = owns * int128_t(nValue);
int128_t piece = 1 + ((temp - 1) / int128_t(totalTokens));

Expand Down
37 changes: 17 additions & 20 deletions src/mastercore_sp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
#include "mastercore_log.h"

#include "main.h"
#include "tinyformat.h"
#include "uint256.h"
#include "utiltime.h"

#include <boost/algorithm/string.hpp>
#include <boost/filesystem.hpp>
#include <boost/format.hpp>
#include <boost/lexical_cast.hpp>

#include "leveldb/db.h"
Expand Down Expand Up @@ -44,15 +44,13 @@ unsigned int CMPSPInfo::updateSP(unsigned int propertyID, Entry const &info)
return propertyID;
}

std::string nextIdStr;
unsigned int res = propertyID;

Object spInfo = info.toJSON();

// generate the SP id
string spKey = (boost::format(FORMAT_BOOST_SPKEY) % propertyID).str();
string spKey = strprintf(FORMAT_BOOST_SPKEY, propertyID);
string spValue = write_string(Value(spInfo), false);
string spPrevKey = (boost::format("blk-%s:sp-%d") % info.update_block.ToString() % propertyID).str();
string spPrevKey = strprintf("blk-%s:sp-%d", info.update_block.ToString(), propertyID);
string spPrevValue;

leveldb::WriteBatch commitBatch;
Expand All @@ -70,7 +68,6 @@ unsigned int CMPSPInfo::updateSP(unsigned int propertyID, Entry const &info)

unsigned int CMPSPInfo::putSP(unsigned char ecosystem, Entry const &info)
{
std::string nextIdStr;
unsigned int res = 0;
switch(ecosystem) {
case OMNI_PROPERTY_MSC: // mastercoin ecosystem, MSC: 1, TMSC: 2, First available SP = 3
Expand All @@ -86,17 +83,17 @@ unsigned int CMPSPInfo::putSP(unsigned char ecosystem, Entry const &info)
Object spInfo = info.toJSON();

// generate the SP id
string spKey = (boost::format(FORMAT_BOOST_SPKEY) % res).str();
string spKey = strprintf(FORMAT_BOOST_SPKEY, res);
string spValue = write_string(Value(spInfo), false);
string txIndexKey = (boost::format(FORMAT_BOOST_TXINDEXKEY) % info.txid.ToString() ).str();
string txValue = (boost::format("%d") % res).str();
string txIndexKey = strprintf(FORMAT_BOOST_TXINDEXKEY, info.txid.ToString());
string txValue = strprintf("%d", res);

// sanity checking
string existingEntry;
if (false == pdb->Get(readoptions, spKey, &existingEntry).IsNotFound() && false == boost::equals(spValue, existingEntry)) {
file_log("%s WRITING SP %d TO LEVELDB WHEN A DIFFERENT SP ALREADY EXISTS FOR THAT ID!!!\n", __FUNCTION__, res);
} else if (false == pdb->Get(readoptions, txIndexKey, &existingEntry).IsNotFound() && false == boost::equals(txValue, existingEntry)) {
file_log("%s WRITING INDEX TXID %s : SP %d IS OVERWRITING A DIFFERENT VALUE!!!\n", __FUNCTION__, info.txid.ToString().c_str(), res);
file_log("%s WRITING INDEX TXID %s : SP %d IS OVERWRITING A DIFFERENT VALUE!!!\n", __FUNCTION__, info.txid.ToString(), res);
}

// atomically write both the the SP and the index to the database
Expand All @@ -120,7 +117,7 @@ bool CMPSPInfo::getSP(unsigned int spid, Entry &info)
return true;
}

string spKey = (boost::format(FORMAT_BOOST_SPKEY) % spid).str();
string spKey = strprintf(FORMAT_BOOST_SPKEY, spid);
string spInfoStr;
if (false == pdb->Get(readoptions, spKey, &spInfoStr).ok()) {
return false;
Expand All @@ -145,7 +142,7 @@ bool CMPSPInfo::hasSP(unsigned int spid)
return true;
}

string spKey = (boost::format(FORMAT_BOOST_SPKEY) % spid).str();
string spKey = strprintf(FORMAT_BOOST_SPKEY, spid);
leveldb::Iterator *iter = pdb->NewIterator(readoptions);
iter->Seek(spKey);
bool res = (iter->Valid() && iter->key().compare(spKey) == 0);
Expand All @@ -159,7 +156,7 @@ unsigned int CMPSPInfo::findSPByTX(uint256 const &txid)
{
unsigned int res = 0;

string txIndexKey = (boost::format(FORMAT_BOOST_TXINDEXKEY) % txid.ToString() ).str();
string txIndexKey = strprintf(FORMAT_BOOST_TXINDEXKEY, txid.ToString());
string spidStr;
if (pdb->Get(readoptions, txIndexKey, &spidStr).ok()) {
res = boost::lexical_cast<unsigned int>(spidStr);
Expand All @@ -185,7 +182,7 @@ int CMPSPInfo::popBlock(uint256 const &block_hash)
// need to roll this SP back
if (info.update_block == info.creation_block) {
// this is the block that created this SP, so delete the SP and the tx index entry
string txIndexKey = (boost::format(FORMAT_BOOST_TXINDEXKEY) % info.txid.ToString() ).str();
string txIndexKey = strprintf(FORMAT_BOOST_TXINDEXKEY, info.txid.ToString());
commitBatch.Delete(iter->key());
commitBatch.Delete(txIndexKey);
} else {
Expand All @@ -194,7 +191,7 @@ int CMPSPInfo::popBlock(uint256 const &block_hash)
boost::split(vstr, key, boost::is_any_of("-"), token_compress_on);
unsigned int propertyID = boost::lexical_cast<unsigned int>(vstr[1]);

string spPrevKey = (boost::format("blk-%s:sp-%d") % info.update_block.ToString() % propertyID).str();
string spPrevKey = strprintf("blk-%s:sp-%d", info.update_block.ToString(), propertyID);
string spPrevValue;

if (false == pdb->Get(readoptions, spPrevKey, &spPrevValue).IsNotFound()) {
Expand Down Expand Up @@ -371,7 +368,7 @@ bool mastercore::isCrowdsalePurchase(uint256 txid, string address, int64_t *prop
for(it = database.begin(); it != database.end(); it++)
{
string tmpTxid = it->first; //uint256 txid = it->first;
string compTxid = txid.GetHex().c_str(); //convert to string to compare since this is how stored in levelDB
string compTxid = txid.GetHex(); //convert to string to compare since this is how stored in levelDB
if (tmpTxid == compTxid)
{
int64_t tmpUserTokens = it->second.at(2);
Expand Down Expand Up @@ -401,7 +398,7 @@ bool mastercore::isCrowdsalePurchase(uint256 txid, string address, int64_t *prop
for(it = database.begin(); it != database.end(); it++)
{
string tmpTxid = it->first; //uint256 txid = it->first;
string compTxid = txid.GetHex().c_str(); //convert to string to compare since this is how stored in levelDB
string compTxid = txid.GetHex(); //convert to string to compare since this is how stored in levelDB
if (tmpTxid == compTxid)
{
int64_t tmpUserTokens = it->second.at(2);
Expand All @@ -427,7 +424,7 @@ bool mastercore::isCrowdsalePurchase(uint256 txid, string address, int64_t *prop
for(it = database.begin(); it != database.end(); it++)
{
string tmpTxid = it->first; //uint256 txid = it->first;
string compTxid = txid.GetHex().c_str(); //convert to string to compare since this is how stored in levelDB
string compTxid = txid.GetHex(); //convert to string to compare since this is how stored in levelDB
if (tmpTxid == compTxid)
{
int64_t tmpUserTokens = it->second.at(2);
Expand All @@ -453,7 +450,7 @@ void mastercore::eraseMaxedCrowdsale(const string &address, uint64_t blockTime,
if (it != my_crowds.end()) {

CMPCrowd &crowd = it->second;
file_log("%s() FOUND MAXED OUT CROWDSALE from address= '%s', erasing...\n", __FUNCTION__, address.c_str());
file_log("%s() FOUND MAXED OUT CROWDSALE from address= '%s', erasing...\n", __FUNCTION__, address);

dumpCrowdsaleInfo(address, crowd);

Expand Down Expand Up @@ -489,7 +486,7 @@ CrowdMap::iterator my_it = my_crowds.begin();

if (blockTime > (int64_t)crowd.getDeadline())
{
file_log("%s() FOUND EXPIRED CROWDSALE from address= '%s', erasing...\n", __FUNCTION__, (my_it->first).c_str());
file_log("%s() FOUND EXPIRED CROWDSALE from address= '%s', erasing...\n", __FUNCTION__, my_it->first);

// TODO: dump the info about this crowdsale being delete into a TXT file (JSON perhaps)
dumpCrowdsaleInfo(my_it->first, my_it->second, true);
Expand Down
Loading

0 comments on commit 0e397bd

Please sign in to comment.