Skip to content
Snippets Groups Projects
Commit 07b46372 authored by Vadim Savchuk's avatar Vadim Savchuk
Browse files

Fixed bug EXT-6798 (Crash in LLTeleportHistoryStorage).

Reason: Attempt to purge empty teleport history resulted in inconsistent history state.
There were two consequences:
1) Further teleports would not properly update the history.
2) Subscribers of history changes were notified of the invalid change and that led to the crash.

My changes:
- added a sanity check to LLTeleportHistoryStorage::onTeleportHistoryChange() to prevent the crash
- purging empty teleport history now does nothing, hence the history doesn't become inconsistent
- removed a redundant (but harmless) call to onTeleportHistoryChanged() from LLNavigationBar::draw() because it's called by LLTeleportHistory::purgeItems() anyway

Reviewed by Mike at https://codereview.productengine.com/secondlife/r/304/

--HG--
branch : product-engine
parent 1186e6dd
No related branches found
No related tags found
No related merge requests found
...@@ -375,7 +375,6 @@ void LLNavigationBar::draw() ...@@ -375,7 +375,6 @@ void LLNavigationBar::draw()
if(mPurgeTPHistoryItems) if(mPurgeTPHistoryItems)
{ {
LLTeleportHistory::getInstance()->purgeItems(); LLTeleportHistory::getInstance()->purgeItems();
onTeleportHistoryChanged();
mPurgeTPHistoryItems = false; mPurgeTPHistoryItems = false;
} }
......
...@@ -136,6 +136,7 @@ void LLTeleportHistory::updateCurrentLocation(const LLVector3d& new_pos) ...@@ -136,6 +136,7 @@ void LLTeleportHistory::updateCurrentLocation(const LLVector3d& new_pos)
if (mCurrentItem < 0 || mCurrentItem >= (int) mItems.size()) // sanity check if (mCurrentItem < 0 || mCurrentItem >= (int) mItems.size()) // sanity check
{ {
llwarns << "Invalid current item. (this should not happen)" << llendl; llwarns << "Invalid current item. (this should not happen)" << llendl;
llassert(!"Invalid current teleport histiry item");
return; return;
} }
LLVector3 new_pos_local = gAgent.getPosAgentFromGlobal(new_pos); LLVector3 new_pos_local = gAgent.getPosAgentFromGlobal(new_pos);
...@@ -166,6 +167,17 @@ void LLTeleportHistory::onHistoryChanged() ...@@ -166,6 +167,17 @@ void LLTeleportHistory::onHistoryChanged()
void LLTeleportHistory::purgeItems() void LLTeleportHistory::purgeItems()
{ {
if (mItems.size() == 0) // no entries yet (we're called before login)
{
// If we don't return here the history will get into inconsistent state, hence:
// 1) updateCurrentLocation() will malfunction,
// so further teleports will not properly update the history;
// 2) mHistoryChangedSignal subscribers will be notified
// of such an invalid change. (EXT-6798)
// Both should not happen.
return;
}
if (mItems.size() > 0) if (mItems.size() > 0)
{ {
mItems.erase(mItems.begin(), mItems.end()-1); mItems.erase(mItems.begin(), mItems.end()-1);
......
...@@ -89,6 +89,13 @@ void LLTeleportHistoryStorage::onTeleportHistoryChange() ...@@ -89,6 +89,13 @@ void LLTeleportHistoryStorage::onTeleportHistoryChange()
if (!th) if (!th)
return; return;
// Hacky sanity check. (EXT-6798)
if (th->getItems().size() == 0)
{
llassert(!"Inconsistent teleport history state");
return;
}
const LLTeleportHistoryItem &item = th->getItems()[th->getCurrentItemIndex()]; const LLTeleportHistoryItem &item = th->getItems()[th->getCurrentItemIndex()];
addItem(item.mTitle, item.mGlobalPos); addItem(item.mTitle, item.mGlobalPos);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment