- removed calls to rsfiles->get{Download,Partials}Directory() in RsDiscSpace class, since it would trigger a call to ftController

- added a lock into ftTransferModule::recvFileData() (Crash reported by Costa due to storing data in a deleted transfer module)
- changed names of functions in ftTransferModules to locked_* when appropriate (helps debugging)



git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@4612 b45a01b8-16f6-495d-af2f-9b41ad6348cc
This commit is contained in:
csoler 2011-09-23 21:08:11 +00:00
parent 3ed0c1d885
commit 97ed1d534f
5 changed files with 75 additions and 57 deletions

View File

@ -1499,6 +1499,8 @@ bool ftController::setDownloadDirectory(std::string path)
RsStackMutex stack(ctrlMutex); /******* LOCKED ********/ RsStackMutex stack(ctrlMutex); /******* LOCKED ********/
mDownloadPath = RsDirUtil::convertPathToUnix(path); mDownloadPath = RsDirUtil::convertPathToUnix(path);
RsDiscSpace::setDownloadPath(mDownloadPath) ;
#ifdef CONTROL_DEBUG #ifdef CONTROL_DEBUG
std::cerr << "ftController::setDownloadDirectory() Okay!"; std::cerr << "ftController::setDownloadDirectory() Okay!";
std::cerr << std::endl; std::cerr << std::endl;
@ -1547,6 +1549,8 @@ bool ftController::setPartialsDirectory(std::string path)
mPartialsPath = path; mPartialsPath = path;
RsDiscSpace::setPartialsPath(path) ;
#if 0 /*** FIX ME !!!**************/ #if 0 /*** FIX ME !!!**************/
/* move all existing files! */ /* move all existing files! */
std::map<std::string, ftFileControl>::iterator it; std::map<std::string, ftFileControl>::iterator it;

View File

@ -91,7 +91,10 @@ ftTransferModule::ftTransferModule(ftFileCreator *fc, ftDataMultiplex *dm, ftCon
} }
ftTransferModule::~ftTransferModule() ftTransferModule::~ftTransferModule()
{} {
// Prevents deletion while called from another thread.
RsStackMutex stack(tfMtx); /******* STACK LOCKED ******/
}
bool ftTransferModule::setFileSources(const std::list<std::string>& peerIds) bool ftTransferModule::setFileSources(const std::list<std::string>& peerIds)
@ -278,6 +281,7 @@ uint32_t ftTransferModule::getDataRate(const std::string& peerId)
//interface to client module //interface to client module
bool ftTransferModule::recvFileData(const std::string& peerId, uint64_t offset, uint32_t chunk_size, void *data) bool ftTransferModule::recvFileData(const std::string& peerId, uint64_t offset, uint32_t chunk_size, void *data)
{ {
RsStackMutex stack(tfMtx); /******* STACK LOCKED ******/
#ifdef FT_DEBUG #ifdef FT_DEBUG
std::cerr << "ftTransferModule::recvFileData()"; std::cerr << "ftTransferModule::recvFileData()";
std::cerr << " peerId: " << peerId; std::cerr << " peerId: " << peerId;
@ -289,9 +293,6 @@ bool ftTransferModule::recvFileData(const std::string& peerId, uint64_t offset,
bool ok = false; bool ok = false;
{
RsStackMutex stack(tfMtx); /******* STACK LOCKED ******/
std::map<std::string,peerInfo>::iterator mit; std::map<std::string,peerInfo>::iterator mit;
mit = mFileSources.find(peerId); mit = mFileSources.find(peerId);
@ -306,16 +307,13 @@ bool ftTransferModule::recvFileData(const std::string& peerId, uint64_t offset,
} }
ok = locked_recvPeerData(mit->second, offset, chunk_size, data); ok = locked_recvPeerData(mit->second, offset, chunk_size, data);
} /***** STACK MUTEX END ****/ locked_storeData(offset, chunk_size, data);
if (ok)
storeData(offset, chunk_size, data);
free(data) ; free(data) ;
return ok; return ok;
} }
void ftTransferModule::requestData(const std::string& peerId, uint64_t offset, uint32_t chunk_size) void ftTransferModule::locked_requestData(const std::string& peerId, uint64_t offset, uint32_t chunk_size)
{ {
#ifdef FT_DEBUG #ifdef FT_DEBUG
std::cerr << "ftTransferModule::requestData()"; std::cerr << "ftTransferModule::requestData()";
@ -330,10 +328,10 @@ void ftTransferModule::requestData(const std::string& peerId, uint64_t offset, u
mMultiplexor->sendDataRequest(peerId, mHash, mSize, offset,chunk_size); mMultiplexor->sendDataRequest(peerId, mHash, mSize, offset,chunk_size);
} }
bool ftTransferModule::getChunk(const std::string& peer_id,uint32_t size_hint,uint64_t &offset, uint32_t &chunk_size) bool ftTransferModule::locked_getChunk(const std::string& peer_id,uint32_t size_hint,uint64_t &offset, uint32_t &chunk_size)
{ {
#ifdef FT_DEBUG #ifdef FT_DEBUG
std::cerr << "ftTransferModule::getChunk()"; std::cerr << "ftTransferModule::locked_getChunk()";
std::cerr << " hash: " << mHash; std::cerr << " hash: " << mHash;
std::cerr << " size: " << mSize; std::cerr << " size: " << mSize;
std::cerr << " offset: " << offset; std::cerr << " offset: " << offset;
@ -352,7 +350,7 @@ bool ftTransferModule::getChunk(const std::string& peer_id,uint32_t size_hint,ui
#ifdef FT_DEBUG #ifdef FT_DEBUG
if (val) if (val)
{ {
std::cerr << "ftTransferModule::getChunk()"; std::cerr << "ftTransferModule::locked_getChunk()";
std::cerr << " Answer: Chunk Available"; std::cerr << " Answer: Chunk Available";
std::cerr << " hash: " << mHash; std::cerr << " hash: " << mHash;
std::cerr << " size: " << mSize; std::cerr << " size: " << mSize;
@ -363,7 +361,7 @@ bool ftTransferModule::getChunk(const std::string& peer_id,uint32_t size_hint,ui
} }
else else
{ {
std::cerr << "ftTransferModule::getChunk()"; std::cerr << "ftTransferModule::locked_getChunk()";
std::cerr << " Answer: No Chunk Available"; std::cerr << " Answer: No Chunk Available";
std::cerr << " peer map needed = " << source_peer_map_needed << std::endl ; std::cerr << " peer map needed = " << source_peer_map_needed << std::endl ;
std::cerr << std::endl; std::cerr << std::endl;
@ -373,7 +371,7 @@ bool ftTransferModule::getChunk(const std::string& peer_id,uint32_t size_hint,ui
return val; return val;
} }
bool ftTransferModule::storeData(uint64_t offset, uint32_t chunk_size,void *data) bool ftTransferModule::locked_storeData(uint64_t offset, uint32_t chunk_size,void *data)
{ {
#ifdef FT_DEBUG #ifdef FT_DEBUG
std::cerr << "ftTransferModule::storeData()"; std::cerr << "ftTransferModule::storeData()";
@ -391,7 +389,6 @@ bool ftTransferModule::queryInactive()
{ {
/* NB: Not sure about this lock... might cause deadlock. /* NB: Not sure about this lock... might cause deadlock.
*/ */
{
RsStackMutex stack(tfMtx); /******* STACK LOCKED ******/ RsStackMutex stack(tfMtx); /******* STACK LOCKED ******/
#ifdef FT_DEBUG #ifdef FT_DEBUG
@ -421,7 +418,6 @@ bool ftTransferModule::queryInactive()
mFileStatus.stat = ftFileStatus::PQIFILE_CHECKING ; mFileStatus.stat = ftFileStatus::PQIFILE_CHECKING ;
mFlag = FT_TM_FLAG_CHECKING; mFlag = FT_TM_FLAG_CHECKING;
} }
}
return true; return true;
} }
@ -948,12 +944,12 @@ bool ftTransferModule::locked_tickPeerTransfer(peerInfo &info)
uint64_t req_offset = 0; uint64_t req_offset = 0;
uint32_t req_size =0 ; uint32_t req_size =0 ;
if (getChunk(info.peerId,next_req,req_offset,req_size)) if (locked_getChunk(info.peerId,next_req,req_offset,req_size))
{ {
if (req_size > 0) if (req_size > 0)
{ {
info.state = PQIPEER_DOWNLOADING; info.state = PQIPEER_DOWNLOADING;
requestData(info.peerId,req_offset,req_size); locked_requestData(info.peerId,req_offset,req_size);
/* start next rtt measurement */ /* start next rtt measurement */
if (!info.rttActive) if (!info.rttActive)

View File

@ -148,13 +148,12 @@ public:
void addCRC32Map(const CRC32Map& map) ; void addCRC32Map(const CRC32Map& map) ;
//interface to multiplex module //interface to multiplex module
bool recvFileData(const std::string& peerId, uint64_t offset, bool recvFileData(const std::string& peerId, uint64_t offset, uint32_t chunk_size, void *data);
uint32_t chunk_size, void *data); void locked_requestData(const std::string& peerId, uint64_t offset, uint32_t chunk_size);
void requestData(const std::string& peerId, uint64_t offset, uint32_t chunk_size);
//interface to file creator //interface to file creator
bool getChunk(const std::string& peer_id,uint32_t size_hint,uint64_t &offset, uint32_t &chunk_size); bool locked_getChunk(const std::string& peer_id,uint32_t size_hint,uint64_t &offset, uint32_t &chunk_size);
bool storeData(uint64_t offset, uint32_t chunk_size, void *data); bool locked_storeData(uint64_t offset, uint32_t chunk_size, void *data);
int tick(); int tick();

View File

@ -44,6 +44,8 @@ uint32_t RsDiscSpace::_size_limit_mb = 100 ;
uint32_t RsDiscSpace::_current_size[3] = { 10000,10000,10000 } ; uint32_t RsDiscSpace::_current_size[3] = { 10000,10000,10000 } ;
bool RsDiscSpace::_last_res[3] = { true,true,true }; bool RsDiscSpace::_last_res[3] = { true,true,true };
RsMutex RsDiscSpace::_mtx("RsDiscSpace") ; RsMutex RsDiscSpace::_mtx("RsDiscSpace") ;
std::string RsDiscSpace::_partials_path = "" ;
std::string RsDiscSpace::_download_path = "" ;
bool RsDiscSpace::crossSystemDiskStats(const char *file, uint64_t& free_blocks, uint64_t& block_size) bool RsDiscSpace::crossSystemDiskStats(const char *file, uint64_t& free_blocks, uint64_t& block_size)
{ {
@ -111,6 +113,18 @@ bool RsDiscSpace::crossSystemDiskStats(const char *file, uint64_t& free_blocks,
return true ; return true ;
} }
void RsDiscSpace::setDownloadPath(const std::string& path)
{
RsStackMutex m(_mtx) ; // Locked
_download_path = path ;
}
void RsDiscSpace::setPartialsPath(const std::string& path)
{
RsStackMutex m(_mtx) ; // Locked
_partials_path = path ;
}
bool RsDiscSpace::checkForDiscSpace(RsDiscSpace::DiscLocation loc) bool RsDiscSpace::checkForDiscSpace(RsDiscSpace::DiscLocation loc)
{ {
RsStackMutex m(_mtx) ; // Locked RsStackMutex m(_mtx) ; // Locked
@ -133,15 +147,15 @@ bool RsDiscSpace::checkForDiscSpace(RsDiscSpace::DiscLocation loc)
#endif #endif
switch(loc) switch(loc)
{ {
case RS_DOWNLOAD_DIRECTORY: rs = crossSystemDiskStats(rsFiles->getDownloadDirectory().c_str(),free_blocks,block_size) ; case RS_DOWNLOAD_DIRECTORY: rs = crossSystemDiskStats(_download_path.c_str(),free_blocks,block_size) ;
#ifdef DEBUG_RSDISCSPACE #ifdef DEBUG_RSDISCSPACE
std::cerr << " path = " << rsFiles->getDownloadDirectory() << std::endl ; std::cerr << " path = " << _download_path << std::endl ;
#endif #endif
break ; break ;
case RS_PARTIALS_DIRECTORY: rs = crossSystemDiskStats(rsFiles->getPartialsDirectory().c_str(),free_blocks,block_size) ; case RS_PARTIALS_DIRECTORY: rs = crossSystemDiskStats(_partials_path.c_str(),free_blocks,block_size) ;
#ifdef DEBUG_RSDISCSPACE #ifdef DEBUG_RSDISCSPACE
std::cerr << " path = " << rsFiles->getPartialsDirectory() << std::endl ; std::cerr << " path = " << _partials_path << std::endl ;
#endif #endif
break ; break ;

View File

@ -46,6 +46,8 @@ class RsDiscSpace
static void setFreeSpaceLimit(uint32_t mega_bytes) ; static void setFreeSpaceLimit(uint32_t mega_bytes) ;
static uint32_t freeSpaceLimit() ; static uint32_t freeSpaceLimit() ;
static void setPartialsPath(const std::string& path) ;
static void setDownloadPath(const std::string& path) ;
private: private:
static bool crossSystemDiskStats(const char *file, uint64_t& free_blocks, uint64_t& block_size) ; static bool crossSystemDiskStats(const char *file, uint64_t& free_blocks, uint64_t& block_size) ;
@ -55,5 +57,8 @@ class RsDiscSpace
static uint32_t _size_limit_mb ; static uint32_t _size_limit_mb ;
static uint32_t _current_size[3] ; static uint32_t _current_size[3] ;
static bool _last_res[3] ; static bool _last_res[3] ;
static std::string _partials_path ;
static std::string _download_path ;
}; };