From 356e1d55cc4313ea229520e42bdfc14c3fc08b51 Mon Sep 17 00:00:00 2001 From: Mike Antipov Date: Mon, 7 Jun 2010 12:15:23 +0300 Subject: EXT-7441 FIXED Improved condition to give up fetched inventory items in LLInventoryFetchItemsObserver Reason: LLInventoryFetchItemsObserver is used to fetch necessary gesture items. Its logic was based on count of "inventory changed" events. In case of there was too many requests stored item UUIDSs are be removed from queue by mistake - notification can be triggered because some other item is renamed. This issue can appear wherever LLInventoryFetchItemsObserver is used. Fix: improved logic to make decision to give up fetching items. For now it bases on period while waiting items to arrive from server and a number of attempts to repeate waiting. This is a constants of the LLInventoryFetchItemsObserver (10 times by 10 seconds) Tested with test case in JIRA (total count of inventory items - 13,888). Most likely that was a reason of inventory loss (EXT-7503). Reviewed by Brad Payne at https://codereview.productengine.com/secondlife/r/507/ --HG-- branch : product-engine --- indra/newview/llinventoryobserver.cpp | 54 ++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 23 deletions(-) (limited to 'indra/newview/llinventoryobserver.cpp') diff --git a/indra/newview/llinventoryobserver.cpp b/indra/newview/llinventoryobserver.cpp index d2b402fe14..bd35259670 100644 --- a/indra/newview/llinventoryobserver.cpp +++ b/indra/newview/llinventoryobserver.cpp @@ -62,13 +62,9 @@ #include "llsdutil.h" #include -// If the viewer gets a notification, your observer assumes -// that that notification is for itself and then tries to process -// the results. The notification could be for something else (e.g. -// you're fetching an item and a notification gets triggered because -// you renamed some other item). This counter is to specify how many -// notification to wait for before giving up. -static const U32 MAX_NUM_NOTIFICATIONS_TO_PROCESS = 127; +const U32 LLInventoryFetchItemsObserver::MAX_NUM_ATTEMPTS_TO_PROCESS = 10; +const F32 LLInventoryFetchItemsObserver::FETCH_TIMER_EXPIRY = 10.0f; + LLInventoryObserver::LLInventoryObserver() { @@ -149,7 +145,7 @@ void LLInventoryCompletionObserver::watchItem(const LLUUID& id) LLInventoryFetchItemsObserver::LLInventoryFetchItemsObserver(const LLUUID& item_id) : LLInventoryFetchObserver(item_id), - mNumTries(MAX_NUM_NOTIFICATIONS_TO_PROCESS) + mNumTries(MAX_NUM_ATTEMPTS_TO_PROCESS) { mIDs.clear(); mIDs.push_back(item_id); @@ -158,35 +154,47 @@ LLInventoryFetchItemsObserver::LLInventoryFetchItemsObserver(const LLUUID& item_ LLInventoryFetchItemsObserver::LLInventoryFetchItemsObserver(const uuid_vec_t& item_ids) : LLInventoryFetchObserver(item_ids), - mNumTries(MAX_NUM_NOTIFICATIONS_TO_PROCESS) + mNumTries(MAX_NUM_ATTEMPTS_TO_PROCESS) { } void LLInventoryFetchItemsObserver::changed(U32 mask) { - BOOL any_items_missing = FALSE; - // scan through the incomplete items and move or erase them as // appropriate. if (!mIncomplete.empty()) { + // if period of an attempt expired... + if (mFetchingPeriod.hasExpired()) + { + // ...reset timer and reduce count of attempts + mFetchingPeriod.reset(); + mFetchingPeriod.setTimerExpirySec(FETCH_TIMER_EXPIRY); + + --mNumTries; + + LL_INFOS("InventoryFetch") << "LLInventoryFetchItemsObserver: " << this << ", attempt(s) left: " << (S32)mNumTries << LL_ENDL; + } + + // do we still have any attempts? + bool timeout_expired = mNumTries <= 0; + for (uuid_vec_t::iterator it = mIncomplete.begin(); it < mIncomplete.end(); ) { const LLUUID& item_id = (*it); LLViewerInventoryItem* item = gInventory.getItem(item_id); if (!item) { - any_items_missing = TRUE; - if (mNumTries > 0) + if (timeout_expired) { - // Keep trying. - ++it; + // Just concede that this item hasn't arrived in reasonable time and continue on. + LL_WARNS("InventoryFetch") << "Fetcher timed out when fetching inventory item UUID: " << item_id << LL_ENDL; + it = mIncomplete.erase(it); } else { - // Just concede that this item hasn't arrived in reasonable time and continue on. - llwarns << "Fetcher timed out when fetching inventory item assetID:" << item_id << llendl; - it = mIncomplete.erase(it); + // Keep trying. + ++it; } continue; } @@ -198,14 +206,10 @@ void LLInventoryFetchItemsObserver::changed(U32 mask) } ++it; } - if (any_items_missing) - { - mNumTries--; - } if (mIncomplete.empty()) { - mNumTries = MAX_NUM_NOTIFICATIONS_TO_PROCESS; + mNumTries = MAX_NUM_ATTEMPTS_TO_PROCESS; done(); } } @@ -315,6 +319,10 @@ void LLInventoryFetchItemsObserver::startFetch() item_entry["item_id"] = (*it); items_llsd.append(item_entry); } + + mFetchingPeriod.resetWithExpiry(FETCH_TIMER_EXPIRY); + mNumTries = MAX_NUM_ATTEMPTS_TO_PROCESS; + fetch_items_from_llsd(items_llsd); } -- cgit v1.2.3 From 823ae2ea98c7fd24053d7bc97fbc178b654190b4 Mon Sep 17 00:00:00 2001 From: Loren Shih Date: Wed, 16 Jun 2010 13:35:57 -0400 Subject: EXT-7735 FIXED When receiving a folder, "Show" (or hotlink) doesn't do anything Fixed highlight and open item code to work for both categories and items (was previously only working for items). --- indra/newview/llinventoryobserver.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'indra/newview/llinventoryobserver.cpp') diff --git a/indra/newview/llinventoryobserver.cpp b/indra/newview/llinventoryobserver.cpp index bd35259670..0ac8fbcb15 100644 --- a/indra/newview/llinventoryobserver.cpp +++ b/indra/newview/llinventoryobserver.cpp @@ -308,7 +308,16 @@ void LLInventoryFetchItemsObserver::startFetch() // assume it's agent inventory. owner_id = gAgent.getID(); } - + + // Ignore categories since they're not items. We + // could also just add this to mComplete but not sure what the + // side-effects would be, so ignoring to be safe. + LLViewerInventoryCategory* cat = gInventory.getCategory(*it); + if (cat) + { + continue; + } + // It's incomplete, so put it on the incomplete container, and // pack this on the message. mIncomplete.push_back(*it); -- cgit v1.2.3 From ca816088999d130b2f36a287b1d33d02cea6ce30 Mon Sep 17 00:00:00 2001 From: Mike Antipov Date: Mon, 21 Jun 2010 11:08:31 +0300 Subject: EXT-7921 FIXED Added a check whether message is what we expecting in LLInventoryAddItemByAssetObserver::changed. Reviewed by Richard Nelson at https://codereview.productengine.com/secondlife/r/618/ --HG-- branch : product-engine --- indra/newview/llinventoryobserver.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'indra/newview/llinventoryobserver.cpp') diff --git a/indra/newview/llinventoryobserver.cpp b/indra/newview/llinventoryobserver.cpp index 0ac8fbcb15..8cb263d9a7 100644 --- a/indra/newview/llinventoryobserver.cpp +++ b/indra/newview/llinventoryobserver.cpp @@ -523,8 +523,14 @@ void LLInventoryAddItemByAssetObserver::changed(U32 mask) return; } - LLPointer item = new LLViewerInventoryItem; LLMessageSystem* msg = gMessageSystem; + if (!(msg->getMessageName() && (0 == strcmp(msg->getMessageName(), "UpdateCreateInventoryItem")))) + { + // this is not our message + return; // to prevent a crash. EXT-7921; + } + + LLPointer item = new LLViewerInventoryItem; S32 num_blocks = msg->getNumberOfBlocksFast(_PREHASH_InventoryData); for(S32 i = 0; i < num_blocks; ++i) { -- cgit v1.2.3