[solved] 3387: "Resolving Combat" crashes freeoriond

Problems and solutions for installing or running FreeOrion, including discussion of bugs if needed before posting a bug report on GitHub. For problems building from source, post in Compile.

Moderators: Oberlus, Oberlus

Post Reply
Message
Author
jsena
Space Floater
Posts: 36
Joined: Tue Feb 16, 2010 4:12 pm

[solved] 3387: "Resolving Combat" crashes freeoriond

#1 Post by jsena » Wed Mar 17, 2010 3:32 pm

I am running Mac OS X 10.4.11
FreeOrion is build 3387

I crash freeorion by doing the following:
->Create a new game with 10 stars and 1 AI player
-> end turn

The screen goes through to "Resolving Combat" and locks up there. If I create a larger galaxy, the game plays fine until there is a fight, at which point it locks up.
freeoriond.crash.log
freeoriond log
(9.38 KiB) Downloaded 54 times
freeoriond.log
(18.36 KiB) Downloaded 50 times
Last edited by jsena on Fri Mar 19, 2010 7:12 pm, edited 2 times in total.

User avatar
.Id
Space Squid
Posts: 76
Joined: Fri Feb 06, 2009 6:54 pm

Re: 3387: "Resolving Combat" crashes freeoriond

#2 Post by .Id » Wed Mar 17, 2010 8:25 pm

I see the same behavior.

The bug looks like it's in UniverseObject::ClearOwners().

That routine is meant to erase all elements of std::set<int> m_owners, but I think that the iterator is invalidated after erasing an element from the set and shouldn't be incremented.

I changed it to just use std::set::clear() instead and it seemed to resolve the issue on initial testing.
It looks like a legitimate bug for all OSes and I don't want to wrap the change in #ifdef clauses for Macs even though that's the only place it's been reported thus far, so anyone want to confirm before I commit?

Code: Select all

===================================================================
--- universe/UniverseObject.cpp (revision 3387)
+++ universe/UniverseObject.cpp (working copy)
@@ -328,8 +328,8 @@
 
 void UniverseObject::ClearOwners()
 {
-    for (std::set<int>::iterator it = m_owners.begin(); it != m_owners.end(); ++it)
-        RemoveOwner(*it);
+    m_owners.clear();
+    StateChangedSignal();
 }
 
 void UniverseObject::SetSystem(int sys)

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 12675
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: 3387: "Resolving Combat" crashes freeoriond

#3 Post by Geoff the Medio » Wed Mar 17, 2010 9:36 pm

.Id wrote:

Code: Select all

void UniverseObject::ClearOwners()
 {
-    for (std::set<int>::iterator it = m_owners.begin(); it != m_owners.end(); ++it)
-        RemoveOwner(*it);
+    m_owners.clear();
+    StateChangedSignal();
 }
Although UniverseObject::RemoveOwner just erases the indicated owner id from the object (and emits a StateChangedSignal if the indicated id was actually an owner), the function is declared virtual. As such, derived classes could do something more interesting, and it's not really correct to just replace the looped calls to RemoveOwner with a single m_owners.clear.

Instead, make a local copy of m_owners at the start of ClearOwners and iterate through that to call RemoveOwner on each id.

Code: Select all

void UniverseObject::ClearOwners()
{
    const std::set<int> initial_owners = m_owners;
    for (std::set<int>::const_iterator it = initial_owners.begin(); it != initial_owners.end(); ++it)
        RemoveOwner(*it);
}
Edit:Committed fix (hopefully)/Edit

jsena
Space Floater
Posts: 36
Joined: Tue Feb 16, 2010 4:12 pm

Re: 3387: "Resolving Combat" crashes freeoriond

#4 Post by jsena » Thu Mar 18, 2010 5:03 pm

That seems to have resolved the problem. Thanks!

User avatar
.Id
Space Squid
Posts: 76
Joined: Fri Feb 06, 2009 6:54 pm

Re: 3387: "Resolving Combat" crashes freeoriond

#5 Post by .Id » Thu Mar 18, 2010 6:02 pm

That solution makes sense to me, Geoff. Thanks.

Post Reply