From 58bb4116dbe4a8afd5df1a7d8db074b0a43fc8f8 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Wed, 10 Feb 2016 19:56:08 +0200 Subject: MAINT-6066 crash in LLTransferSource::getID() --- indra/llmessage/llassetstorage.cpp | 122 +++++++++++++++++------------- indra/llmessage/llassetstorage.h | 112 +++++++++++---------------- indra/llmessage/llhttpassetstorage.cpp | 48 ++++++------ indra/llmessage/lltransfertargetvfile.cpp | 38 +++++++--- indra/llmessage/lltransfertargetvfile.h | 6 +- 5 files changed, 167 insertions(+), 159 deletions(-) (limited to 'indra/llmessage') diff --git a/indra/llmessage/llassetstorage.cpp b/indra/llmessage/llassetstorage.cpp index 11505fcc75..cab3073eca 100755 --- a/indra/llmessage/llassetstorage.cpp +++ b/indra/llmessage/llassetstorage.cpp @@ -157,29 +157,50 @@ void LLAssetInfo::setFromNameValue( const LLNameValue& nv ) LL_DEBUGS("AssetStorage") << "creator: " << mCreatorID << LL_ENDL; } +///---------------------------------------------------------------------------- +/// LLBaseDownloadRequest +///---------------------------------------------------------------------------- + +LLBaseDownloadRequest::LLBaseDownloadRequest(const LLUUID &uuid, const LLAssetType::EType type) +: mUUID(uuid), +mType(type), +mDownCallback(NULL), +mUserData(NULL), +mHost(), +mIsTemp(FALSE), +mIsPriority(FALSE), +mDataSentInFirstPacket(FALSE), +mDataIsInVFS(FALSE) +{ + // Need to guarantee that this time is up to date, we may be creating a circuit even though we haven't been + // running a message system loop. + mTime = LLMessageSystem::getMessageTimeSeconds(TRUE); +} + +// virtual +LLBaseDownloadRequest::~LLBaseDownloadRequest() +{ +} + +// virtual +LLBaseDownloadRequest* LLBaseDownloadRequest::getCopy() +{ + return new LLBaseDownloadRequest(*this); +} + + ///---------------------------------------------------------------------------- /// LLAssetRequest ///---------------------------------------------------------------------------- LLAssetRequest::LLAssetRequest(const LLUUID &uuid, const LLAssetType::EType type) -: mUUID(uuid), - mType(type), - mDownCallback( NULL ), +: LLBaseDownloadRequest(uuid, type), mUpCallback( NULL ), mInfoCallback( NULL ), - mUserData( NULL ), - mHost(), - mIsTemp( FALSE ), mIsLocal(FALSE), mIsUserWaiting(FALSE), - mTimeout(LL_ASSET_STORAGE_TIMEOUT), - mIsPriority(FALSE), - mDataSentInFirstPacket(FALSE), - mDataIsInVFS( FALSE ) + mTimeout(LL_ASSET_STORAGE_TIMEOUT) { - // Need to guarantee that this time is up to date, we may be creating a circuit even though we haven't been - // running a message system loop. - mTime = LLMessageSystem::getMessageTimeSeconds(TRUE); } // virtual @@ -217,56 +238,51 @@ LLSD LLAssetRequest::getFullDetails() const return sd; } +LLBaseDownloadRequest* LLAssetRequest::getCopy() +{ + return new LLAssetRequest(*this); +} + ///---------------------------------------------------------------------------- /// LLInvItemRequest ///---------------------------------------------------------------------------- LLInvItemRequest::LLInvItemRequest(const LLUUID &uuid, const LLAssetType::EType type) -: mUUID(uuid), - mType(type), - mDownCallback( NULL ), - mUserData( NULL ), - mHost(), - mIsTemp( FALSE ), - mIsPriority(FALSE), - mDataSentInFirstPacket(FALSE), - mDataIsInVFS( FALSE ) +: LLBaseDownloadRequest(uuid, type) { - // Need to guarantee that this time is up to date, we may be creating a circuit even though we haven't been - // running a message system loop. - mTime = LLMessageSystem::getMessageTimeSeconds(TRUE); } +// virtual LLInvItemRequest::~LLInvItemRequest() { } +LLBaseDownloadRequest* LLInvItemRequest::getCopy() +{ + return new LLInvItemRequest(*this); +} + ///---------------------------------------------------------------------------- /// LLEstateAssetRequest ///---------------------------------------------------------------------------- LLEstateAssetRequest::LLEstateAssetRequest(const LLUUID &uuid, const LLAssetType::EType atype, EstateAssetType etype) -: mUUID(uuid), - mAType(atype), - mEstateAssetType(etype), - mDownCallback( NULL ), - mUserData( NULL ), - mHost(), - mIsTemp( FALSE ), - mIsPriority(FALSE), - mDataSentInFirstPacket(FALSE), - mDataIsInVFS( FALSE ) -{ - // Need to guarantee that this time is up to date, we may be creating a circuit even though we haven't been - // running a message system loop. - mTime = LLMessageSystem::getMessageTimeSeconds(TRUE); +: LLBaseDownloadRequest(uuid, atype), + mEstateAssetType(etype) +{ } +// Virtual LLEstateAssetRequest::~LLEstateAssetRequest() { } +LLBaseDownloadRequest* LLEstateAssetRequest::getCopy() +{ + return new LLEstateAssetRequest(*this); +} + ///---------------------------------------------------------------------------- /// LLAssetStorage @@ -565,7 +581,7 @@ void LLAssetStorage::_queueDataRequest(const LLUUID& uuid, LLAssetType::EType at // Set our destination file, and the completion callback. LLTransferTargetParamsVFile tpvf; tpvf.setAsset(uuid, atype); - tpvf.setCallback(downloadCompleteCallback, req); + tpvf.setCallback(downloadCompleteCallback, *req); //LL_INFOS() << "Starting transfer for " << uuid << LL_ENDL; LLTransferTargetChannel *ttcp = gTransferManager.getTargetChannel(mUpstreamHost, LLTCT_ASSET); @@ -589,7 +605,7 @@ void LLAssetStorage::downloadCompleteCallback( S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, LLExtStat ext_status) + LLBaseDownloadRequest* user_data, LLExtStat ext_status) { LL_DEBUGS("AssetStorage") << "ASSET_TRACE asset " << file_id << " downloadCompleteCallback" << LL_ENDL; @@ -619,14 +635,12 @@ void LLAssetStorage::downloadCompleteCallback( if (download_iter != gAssetStorage->mPendingDownloads.end()) { - // either has already been deleted by _cleanupRequests (as result req becomes invalid) - // or it's a transfer. callback_id = file_id; callback_type = file_type; } else { - // we will be deleting elements from mPendingDownloads which req should be part of, save id and type + // either has already been deleted by _cleanupRequests or it's a transfer. callback_id = req->getUUID(); callback_type = req->getType(); } @@ -737,10 +751,10 @@ void LLAssetStorage::getEstateAsset(const LLHost &object_sim, const LLUUID &agen if (source_host.isOk()) { // stash the callback info so we can find it after we get the response message - LLEstateAssetRequest *req = new LLEstateAssetRequest(asset_id, atype, etype); - req->mDownCallback = callback; - req->mUserData = user_data; - req->mIsPriority = is_priority; + LLEstateAssetRequest req(asset_id, atype, etype); + req.mDownCallback = callback; + req.mUserData = user_data; + req.mIsPriority = is_priority; // send request message to our upstream data provider // Create a new asset transfer. @@ -774,7 +788,7 @@ void LLAssetStorage::downloadEstateAssetCompleteCallback( S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, + LLBaseDownloadRequest* user_data, LLExtStat ext_status) { LLEstateAssetRequest *req = (LLEstateAssetRequest*)user_data; @@ -881,10 +895,10 @@ void LLAssetStorage::getInvItemAsset(const LLHost &object_sim, const LLUUID &age if (source_host.isOk()) { // stash the callback info so we can find it after we get the response message - LLInvItemRequest *req = new LLInvItemRequest(asset_id, atype); - req->mDownCallback = callback; - req->mUserData = user_data; - req->mIsPriority = is_priority; + LLInvItemRequest req(asset_id, atype); + req.mDownCallback = callback; + req.mUserData = user_data; + req.mIsPriority = is_priority; // send request message to our upstream data provider // Create a new asset transfer. @@ -922,7 +936,7 @@ void LLAssetStorage::downloadInvItemCompleteCallback( S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, + LLBaseDownloadRequest* user_data, LLExtStat ext_status) { LLInvItemRequest *req = (LLInvItemRequest*)user_data; diff --git a/indra/llmessage/llassetstorage.h b/indra/llmessage/llassetstorage.h index 1bb4acea9e..8a4d41565f 100755 --- a/indra/llmessage/llassetstorage.h +++ b/indra/llmessage/llassetstorage.h @@ -92,38 +92,52 @@ public: }; -class LLAssetRequest +class LLBaseDownloadRequest { public: - LLAssetRequest(const LLUUID &uuid, const LLAssetType::EType at); - virtual ~LLAssetRequest(); - - LLUUID getUUID() const { return mUUID; } - LLAssetType::EType getType() const { return mType; } + LLBaseDownloadRequest(const LLUUID &uuid, const LLAssetType::EType at); + virtual ~LLBaseDownloadRequest(); + + LLUUID getUUID() const { return mUUID; } + LLAssetType::EType getType() const { return mType; } + + void setUUID(const LLUUID& id) { mUUID = id; } + void setType(LLAssetType::EType type) { mType = type; } - void setUUID(const LLUUID& id) { mUUID = id; } - void setType(LLAssetType::EType type) { mType = type; } - void setTimeout (F64Seconds timeout) { mTimeout = timeout; } + virtual LLBaseDownloadRequest* getCopy(); protected: - LLUUID mUUID; - LLAssetType::EType mType; + LLUUID mUUID; + LLAssetType::EType mType; + +public: + void(*mDownCallback)(LLVFS*, const LLUUID&, LLAssetType::EType, void *, S32, LLExtStat); + + void *mUserData; + LLHost mHost; + BOOL mIsTemp; + F64Seconds mTime; // Message system time + BOOL mIsPriority; + BOOL mDataSentInFirstPacket; + BOOL mDataIsInVFS; +}; +class LLAssetRequest : public LLBaseDownloadRequest +{ public: - void (*mDownCallback)(LLVFS*, const LLUUID&, LLAssetType::EType, void *, S32, LLExtStat); + LLAssetRequest(const LLUUID &uuid, const LLAssetType::EType at); + virtual ~LLAssetRequest(); + + void setTimeout(F64Seconds timeout) { mTimeout = timeout; } + + virtual LLBaseDownloadRequest* getCopy(); + void (*mUpCallback)(const LLUUID&, void *, S32, LLExtStat); void (*mInfoCallback)(LLAssetInfo *, void *, S32); - void *mUserData; - LLHost mHost; - BOOL mIsTemp; BOOL mIsLocal; BOOL mIsUserWaiting; // We don't want to try forever if a user is waiting for a result. - F64Seconds mTime; // Message system time F64Seconds mTimeout; // Amount of time before timing out. - BOOL mIsPriority; - BOOL mDataSentInFirstPacket; - BOOL mDataIsInVFS; LLUUID mRequestingAgentID; // Only valid for uploads from an agent virtual LLSD getTerseDetails() const; @@ -141,63 +155,27 @@ struct ll_asset_request_equal : public std::equal_to }; -class LLInvItemRequest +class LLInvItemRequest : public LLBaseDownloadRequest { public: - LLInvItemRequest(const LLUUID &uuid, const LLAssetType::EType at); - virtual ~LLInvItemRequest(); - - LLUUID getUUID() const { return mUUID; } - LLAssetType::EType getType() const { return mType; } - - void setUUID(const LLUUID& id) { mUUID = id; } - void setType(LLAssetType::EType type) { mType = type; } - -protected: - LLUUID mUUID; - LLAssetType::EType mType; - -public: - void (*mDownCallback)(LLVFS*, const LLUUID&, LLAssetType::EType, void *, S32, LLExtStat); - - void *mUserData; - LLHost mHost; - BOOL mIsTemp; - F64Seconds mTime; // Message system time - BOOL mIsPriority; - BOOL mDataSentInFirstPacket; - BOOL mDataIsInVFS; + LLInvItemRequest(const LLUUID &uuid, const LLAssetType::EType at); + virtual ~LLInvItemRequest(); + virtual LLBaseDownloadRequest* getCopy(); }; -class LLEstateAssetRequest +class LLEstateAssetRequest : public LLBaseDownloadRequest { public: - LLEstateAssetRequest(const LLUUID &uuid, const LLAssetType::EType at, EstateAssetType et); - virtual ~LLEstateAssetRequest(); + LLEstateAssetRequest(const LLUUID &uuid, const LLAssetType::EType at, EstateAssetType et); + virtual ~LLEstateAssetRequest(); - LLUUID getUUID() const { return mUUID; } - LLAssetType::EType getAType() const { return mAType; } + LLAssetType::EType getAType() const { return mType; } - void setUUID(const LLUUID& id) { mUUID = id; } - void setType(LLAssetType::EType type) { mAType = type; } + virtual LLBaseDownloadRequest* getCopy(); protected: - LLUUID mUUID; - LLAssetType::EType mAType; EstateAssetType mEstateAssetType; - -public: - void (*mDownCallback)(LLVFS*, const LLUUID&, LLAssetType::EType, void *, S32, LLExtStat); - - void *mUserData; - LLHost mHost; - BOOL mIsTemp; - F64Seconds mTime; // Message system time - BOOL mIsPriority; - BOOL mDataSentInFirstPacket; - BOOL mDataIsInVFS; - }; @@ -369,17 +347,17 @@ public: S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, LLExtStat ext_status); + LLBaseDownloadRequest* user_data, LLExtStat ext_status); static void downloadEstateAssetCompleteCallback( S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, LLExtStat ext_status); + LLBaseDownloadRequest* user_data, LLExtStat ext_status); static void downloadInvItemCompleteCallback( S32 result, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, LLExtStat ext_status); + LLBaseDownloadRequest* user_data, LLExtStat ext_status); // upload process callbacks static void uploadCompleteCallback(const LLUUID&, void *user_data, S32 result, LLExtStat ext_status); diff --git a/indra/llmessage/llhttpassetstorage.cpp b/indra/llmessage/llhttpassetstorage.cpp index e202154445..ace65760c3 100755 --- a/indra/llmessage/llhttpassetstorage.cpp +++ b/indra/llmessage/llhttpassetstorage.cpp @@ -938,22 +938,22 @@ void LLHTTPAssetStorage::checkForTimeouts() long curl_result = 0; S32 xfer_result = LL_ERR_NOERR; - LLHTTPAssetRequest *req = NULL; - curl_easy_getinfo(curl_msg->easy_handle, CURLINFO_PRIVATE, &req); + LLHTTPAssetRequest *http_req = NULL; + curl_easy_getinfo(curl_msg->easy_handle, CURLINFO_PRIVATE, &http_req); // TODO: Throw curl_result at all callbacks. curl_easy_getinfo(curl_msg->easy_handle, CURLINFO_HTTP_CODE, &curl_result); - if (RT_UPLOAD == req->mRequestType || RT_LOCALUPLOAD == req->mRequestType) + if (RT_UPLOAD == http_req->mRequestType || RT_LOCALUPLOAD == http_req->mRequestType) { if (curl_msg->data.result == CURLE_OK && ( curl_result == HTTP_OK || curl_result == HTTP_CREATED || curl_result == HTTP_NO_CONTENT)) { - LL_INFOS() << "Success uploading " << req->getUUID() << " to " << req->mURLBuffer << LL_ENDL; - if (RT_LOCALUPLOAD == req->mRequestType) + LL_INFOS() << "Success uploading " << http_req->getUUID() << " to " << http_req->mURLBuffer << LL_ENDL; + if (RT_LOCALUPLOAD == http_req->mRequestType) { - addTempAssetData(req->getUUID(), req->mRequestingAgentID, mHostName); + addTempAssetData(http_req->getUUID(), http_req->mRequestingAgentID, mHostName); } } else if (curl_msg->data.result == CURLE_COULDNT_CONNECT || @@ -961,18 +961,18 @@ void LLHTTPAssetStorage::checkForTimeouts() curl_result == HTTP_BAD_GATEWAY || curl_result == HTTP_SERVICE_UNAVAILABLE) { - LL_WARNS() << "Re-requesting upload for " << req->getUUID() << ". Received upload error to " << req->mURLBuffer << + LL_WARNS() << "Re-requesting upload for " << http_req->getUUID() << ". Received upload error to " << http_req->mURLBuffer << " with result " << curl_easy_strerror(curl_msg->data.result) << ", http result " << curl_result << LL_ENDL; ////HACK (probably) I am sick of this getting requeued and driving me mad. - //if (req->mIsUserWaiting) + //if (http_req->mIsUserWaiting) //{ - // deletePendingRequest(RT_UPLOAD, req->getType(), req->getUUID()); + // deletePendingRequest(RT_UPLOAD, http_req->getType(), http_req->getUUID()); //} } else { - LL_WARNS() << "Failure uploading " << req->getUUID() << " to " << req->mURLBuffer << + LL_WARNS() << "Failure uploading " << http_req->getUUID() << " to " << http_req->mURLBuffer << " with result " << curl_easy_strerror(curl_msg->data.result) << ", http result " << curl_result << LL_ENDL; xfer_result = LL_ERR_ASSET_REQUEST_FAILED; @@ -985,39 +985,39 @@ void LLHTTPAssetStorage::checkForTimeouts() { // shared upload finished callback // in the base class, this is called from processUploadComplete - _callUploadCallbacks(req->getUUID(), req->getType(), (xfer_result == 0), LL_EXSTAT_CURL_RESULT | curl_result); + _callUploadCallbacks(http_req->getUUID(), http_req->getType(), (xfer_result == 0), LL_EXSTAT_CURL_RESULT | curl_result); // Pending upload flag will get cleared when the request is deleted } } - else if (RT_DOWNLOAD == req->mRequestType) + else if (RT_DOWNLOAD == http_req->mRequestType) { if (curl_result == HTTP_OK && curl_msg->data.result == CURLE_OK) { - if (req->mVFile && req->mVFile->getSize() > 0) + if (http_req->mVFile && http_req->mVFile->getSize() > 0) { - LL_INFOS() << "Success downloading " << req->mURLBuffer << ", size " << req->mVFile->getSize() << LL_ENDL; + LL_INFOS() << "Success downloading " << http_req->mURLBuffer << ", size " << http_req->mVFile->getSize() << LL_ENDL; - req->mVFile->rename(req->getUUID(), req->getType()); + http_req->mVFile->rename(http_req->getUUID(), http_req->getType()); } else { // *TODO: if this actually indicates a bad asset on the server // (not certain at this point), then delete it - LL_WARNS() << "Found " << req->mURLBuffer << " to be zero size" << LL_ENDL; + LL_WARNS() << "Found " << http_req->mURLBuffer << " to be zero size" << LL_ENDL; xfer_result = LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE; } } else { // KLW - TAT See if an avatar owns this texture, and if so request re-upload. - LL_WARNS() << "Failure downloading " << req->mURLBuffer << + LL_WARNS() << "Failure downloading " << http_req->mURLBuffer << " with result " << curl_easy_strerror(curl_msg->data.result) << ", http result " << curl_result << LL_ENDL; xfer_result = (curl_result == HTTP_NOT_FOUND) ? LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE : LL_ERR_ASSET_REQUEST_FAILED; - if (req->mVFile) + if (http_req->mVFile) { - req->mVFile->remove(); + http_req->mVFile->remove(); } } @@ -1025,9 +1025,9 @@ void LLHTTPAssetStorage::checkForTimeouts() // this will cleanup all requests for this asset, including ours downloadCompleteCallback( xfer_result, - req->getUUID(), - req->getType(), - (void *)req, + http_req->getUUID(), + http_req->getType(), + http_req, LL_EXSTAT_CURL_RESULT | curl_result); // Pending download flag will get cleared when the request is deleted } @@ -1038,8 +1038,8 @@ void LLHTTPAssetStorage::checkForTimeouts() } // Deleting clears the pending upload/download flag if it's set and the request is transferring - delete req; - req = NULL; + delete http_req; + http_req = NULL; } } while (curl_msg && queue_length > 0); diff --git a/indra/llmessage/lltransfertargetvfile.cpp b/indra/llmessage/lltransfertargetvfile.cpp index 3c234b9726..a572c68a7f 100755 --- a/indra/llmessage/lltransfertargetvfile.cpp +++ b/indra/llmessage/lltransfertargetvfile.cpp @@ -42,7 +42,7 @@ LLTransferTargetParamsVFile::LLTransferTargetParamsVFile() : LLTransferTargetParams(LLTTT_VFILE), mAssetType(LLAssetType::AT_NONE), mCompleteCallback(NULL), - mUserDatap(NULL), + mRequestDatap(NULL), mErrCode(0) { } @@ -55,10 +55,14 @@ void LLTransferTargetParamsVFile::setAsset( mAssetType = asset_type; } -void LLTransferTargetParamsVFile::setCallback(LLTTVFCompleteCallback cb, void *user_data) +void LLTransferTargetParamsVFile::setCallback(LLTTVFCompleteCallback cb, LLBaseDownloadRequest& request) { mCompleteCallback = cb; - mUserDatap = user_data; + if (mRequestDatap) + { + delete mRequestDatap; + } + mRequestDatap = request.getCopy(); } bool LLTransferTargetParamsVFile::unpackParams(LLDataPacker& dp) @@ -98,6 +102,12 @@ LLTransferTargetVFile::LLTransferTargetVFile( LLTransferTargetVFile::~LLTransferTargetVFile() { + if (mParams.mRequestDatap) + { + // TODO: Consider doing it in LLTransferTargetParamsVFile's destructor + delete mParams.mRequestDatap; + mParams.mRequestDatap = NULL; + } } @@ -208,12 +218,18 @@ void LLTransferTargetVFile::completionCallback(const LLTSCode status) err_code = LL_ERR_ASSET_REQUEST_FAILED; break; } - if (mParams.mCompleteCallback) - { - mParams.mCompleteCallback(err_code, - mParams.getAssetID(), - mParams.getAssetType(), - mParams.mUserDatap, - LL_EXSTAT_NONE); - } + + if (mParams.mRequestDatap) + { + if (mParams.mCompleteCallback) + { + mParams.mCompleteCallback(err_code, + mParams.getAssetID(), + mParams.getAssetType(), + mParams.mRequestDatap, + LL_EXSTAT_NONE); + } + delete mParams.mRequestDatap; + mParams.mRequestDatap = NULL; + } } diff --git a/indra/llmessage/lltransfertargetvfile.h b/indra/llmessage/lltransfertargetvfile.h index 23a65e4bb2..c819c1e2f2 100755 --- a/indra/llmessage/lltransfertargetvfile.h +++ b/indra/llmessage/lltransfertargetvfile.h @@ -39,7 +39,7 @@ typedef void (*LLTTVFCompleteCallback)( S32 status, const LLUUID& file_id, LLAssetType::EType file_type, - void* user_data, LLExtStat ext_status ); + LLBaseDownloadRequest* user_data, LLExtStat ext_status ); class LLTransferTargetParamsVFile : public LLTransferTargetParams { @@ -47,7 +47,7 @@ public: LLTransferTargetParamsVFile(); void setAsset(const LLUUID& asset_id, LLAssetType::EType asset_type); - void setCallback(LLTTVFCompleteCallback cb, void* user_data); + void setCallback(LLTTVFCompleteCallback cb, LLBaseDownloadRequest& request); LLUUID getAssetID() const { return mAssetID; } LLAssetType::EType getAssetType() const { return mAssetType; } @@ -60,7 +60,7 @@ protected: LLAssetType::EType mAssetType; LLTTVFCompleteCallback mCompleteCallback; - void* mUserDatap; + LLBaseDownloadRequest* mRequestDatap; S32 mErrCode; }; -- cgit v1.2.3