1.72 Freeze with conditions shipdata + system.legacy_add...

For test results, bug reports, announcements of new builds etc.

Moderators: winston, another_commander, Getafix

Post Reply
User avatar
Svengali
Commander
Commander
Posts: 2370
Joined: Sat Oct 20, 2007 2:52 pm

1.72 Freeze with conditions shipdata + system.legacy_add...

Post by Svengali »

System: Win

When Oolite gets the command to create a ship, but the shipdata says no (because of the conditions) Oolite freezes and starts to consume a lot of memory in steps 10MB-40MB until the application gets killed.

You can test it by giving the Adder in Oolites shipdata:

Code: Select all

conditions = ("systemGovernment_number greaterthan 7");
This condition will never be true, but is still valid. Then launch from station and try

Code: Select all

system.legacy_addSystemShips('oolite-adder',1,1);
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: 1.72 Freeze with conditions shipdata + system.legacy_add

Post by Eric Walch »

Svengali wrote:
System: Win

When Oolite gets the command to create a ship, but the shipdata says no (because of the conditions) Oolite freezes and starts to consume a lot of memory in steps 10MB-40MB until the application gets killed.
Not on my mac. But this could be related to the crashes with the astomine from commies.oxp That one also uses conditions for all its ships.

And also Trident-Down uses conditions in shipdata. (And ups-courier also uses conditions for some ships)
another_commander
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 6629
Joined: Wed Feb 28, 2007 7:54 am

Re: 1.72 Freeze with conditions shipdata + system.legacy_add

Post by another_commander »

Svengali wrote:
System: Win

When Oolite gets the command to create a ship, but the shipdata says no (because of the conditions) Oolite freezes and starts to consume a lot of memory in steps 10MB-40MB until the application gets killed.
Oh great, another one of those...
*sigh* Confirmed.
bmaxa
Average
Average
Posts: 11
Joined: Thu Nov 06, 2008 2:14 pm
Location: Belgrade, Serbia

Bug solved

Post by bmaxa »

There are few problems in universe.m and probabilitySet.
First method count for pset was used in nsarray not MutableProbabilitySet
returnig incorrect value, and other thing is method removeObject
from ConcreteMutableProbabilitySet never removed object
as condition was if(object != nil)return.
So, if there was not ship which satisfied condition
and random is called that would produce endless loop.

--------------------------------------------------------------------------------
pset = [[[registry probabilitySetForRole:role] mutableCopy] autorelease];
if([pset isKindOfClass: [OOMutableProbabilitySet class]])
{
OOLog(@"bane",@"Starting");

int i = 0;
while ([pset countPS] > 0 && i<1000)
{
++i;
// Select a ship, check conditions and return it if possible.
//shipKey = [registry randomShipKeyForRole:role];
shipKey = [pset randomObject]; // this is correct I think
OOLog(@"bane",@"%@ %d",shipKey,[pset countPS]);
if ([self canInstantiateShip:shipKey]) return shipKey;


// Condition failed -> remove ship from consideration.
[pset removeObject:shipKey];
}
---------------------------------------------------------------------------------
- (void) removeObject:(id)object
{
// if (object != nil) return; caused not to remove object
OOLog(@"bane",@"removing %@",(NSString*)object);
OOUInteger index = [_objects indexOfObject:object];
if(index == NSNotFound)OOLog(@"bane",@"%@ not found!",(NSString*)object);
if (index != NSNotFound)
{
[_objects removeObjectAtIndex:index];
_sumOfWeights -= [_weights floatAtIndex:index];
[_weights removeObjectAtIndex:index];
}
}
------------------------------------------------------------------------------
there is final problem which is that randomObject returns nil
if MutableProbabilitySet already contains objects,
so I have made counter 'i' to avoid endless loop.

TridentDown now loads properly, but it has ships I think
which cannot be instantiated that caused endless loop.

Hope this helps, Branimir.
another_commander
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 6629
Joined: Wed Feb 28, 2007 7:54 am

Post by another_commander »

OK, I think we may hire you... ;-)

The crash that has been torturing us since the release of 1.72 was actually inside OOProbabilitySet.m, -(void) removeObject: (id) object method, as you point out. But instead of just commenting the line out, it should be changed from

Code: Select all

if (object != nil)  return;
to

Code: Select all

if (object == nil)  return;
I have not had the time to test anything else, but for sure Commies.oxp (and most likely any other OXP adding ships with conditions) is not crashing anymore.

Good job bmaxa, looking forward to the next bugfix.
User avatar
Ark
---- E L I T E ----
---- E L I T E ----
Posts: 664
Joined: Sun Dec 09, 2007 8:22 am
Location: Athens Greece

Post by Ark »

I think there must be a “Guardian angel” of some kind for oolite

In 1.70 & 1.71 we had Kaks came out from nowhere killing numerous bugs at no time and now that kaks is missing (Where are you man?? :cry: ) bmaxa came out of the blue doing the same think :shock:

God save oolite man!!!!!
Last edited by Ark on Fri Nov 07, 2008 5:13 pm, edited 1 time in total.
bmaxa
Average
Average
Posts: 11
Joined: Thu Nov 06, 2008 2:14 pm
Location: Belgrade, Serbia

Post by bmaxa »

Thank you I will try my best.

Another thing is I'm compiling on ubuntu 8.10 amd64 and got
these warnings about pset count not used from OOMutableSet
but from NSArray as methods have same name?
I can swear that count was not decreased before I changed name
count to countPS, though perhaps I'm wrong.

Best wishes , Branimir.
another_commander
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 6629
Joined: Wed Feb 28, 2007 7:54 am

Post by another_commander »

bmaxa wrote:
I can swear that count was not decreased before I changed name
count to countPS, though perhaps I'm wrong.
The count was not decreased because no object was ever removed from the probability set. The removeObject method was always returning without doing anything, therefore the count was always the same, therefore endless loop.

In any case, thanks for giving me back my weekend ;-)
bmaxa
Average
Average
Posts: 11
Joined: Thu Nov 06, 2008 2:14 pm
Location: Belgrade, Serbia

Post by bmaxa »

I have returned method name to count and is decreasing anyway,
you are absolutely right. Seems that I first changed method name
to countPS then commented out line ;)

Greets, Branimir.
User avatar
TGHC
---- E L I T E ----
---- E L I T E ----
Posts: 2157
Joined: Mon Jan 31, 2005 4:16 pm
Location: Berkshire, UK

Post by TGHC »

What a great community this is. Keep up the good work guys.
>
>
>
>
>
>
>
>
Pssssst: does anybody know what the hell they are talking about?
The Grey Haired Commander has spoken!
OK so I'm a PC user - "you know whats scary? Out of billions of sperm I was the fastest"
bmaxa
Average
Average
Posts: 11
Joined: Thu Nov 06, 2008 2:14 pm
Location: Belgrade, Serbia

Post by bmaxa »

I'm sorry to bother again but bug is not cleared out with just
if condition in removeObject.
Other part of bug is in shipKey = [registry randomShipKeyForRole:role];
because it will call randomObject alright, but on other set, which
does not guarantees that all object will come out eventualy
as there are objects with zero weights. As expected TridentDown
loops more then 1000- times.

so proper solution for this loop is
--------------------------------------------------------
while ([pset count] > 0)
{
// Select a ship, check conditions and return it if possible.
//shipKey = [registry randomShipKeyForRole:role];
// does not guarantees that all objects will come out
// and spends much more time then necessary
shipKey = [pset randomObject]; // this is correct I think
// as same object will not be returned twice
if(shipKey == nil)
{
break; // there are no more objects with non zero weight
}
if ([self canInstantiateShip:shipKey]) return shipKey;


// Condition failed -> remove ship from consideration.
[pset removeObject:shipKey];
}
-------------------------------------------------------
randomObject will return all objects that don;t have weight zero
but when cumulative weight reaches 0 it will return nil so that
means that we should break from loop.

Greets, Branimir.
Post Reply