Re: bug: unit->busy flag doesn't get cleared

[Hans Ronne <hronne at pp dot sbbs dot se>]

>> I put in a debug warning for units that get their flag cleared at the end
>> of turn. So finding the bug will be easy once you trigger it.
>
>OK, I reproduced it.  Here's an excerpt from the log.  The unit was
>newly created.  Maybe something in make_unit_complete or thereabouts
>has some kind of race condition?

Heh. This debugging did reveal another bug - in my debugging code.
143947984 is not a string. Obviously, %d should be %s.

I take it that the newly created infantry unit is the one that got stalled
(hard to tell from the buggy debugging code). In that case I think I
understand what is going on:

First, we know that you gave it some kind of command, since this is the
only way the busy flag can ever be set.

Second, we know (from the debugging code) that the unit executed
move_one_unit_multiple on the turn it was completed. This means that the
unit had non-zero acp at that point and also that it had either a plan or a
pending action.

Third, we know that it did not have a pending action. If it had, the busy
flag would have been cleared, no matter what.

The only logical conclusion is that the command you gave it failed to
schedule an action, even though the busy flag was set. Checking out the
code, I can see two things:

First, this is indeed possible, since the busy flag is set without checking
if prep_x_action fails or not. This is a bug. The code should check the
result before setting the flag.

Second, the only way prep_x_action can fail is if unit->act does not exist.
This means that the actorstate object of the new unit had not yet been
allocated when you gave the command.

But how is this possible? init_unit_actorstate is called by
make_unit_complete. This means that the unit was not complete yet when you
gave it a command. This should not be. An incomplete unit should never be
able to become the current unit.

In fact, it is possible because of a second bug! Checking out
could_be_next_unit, I see that it tests only for alive(unit) which is true
as soon as the unit has been created. It does not test for is_active(unit)
which is true only when the unit is complete.

In conclusion, my analysis is that the stalled unit was the result of two
interacting bugs: the failure to test the result of prep_x_action before
setting the busy flag, and the fact that could_be_next_unit only tests if a
unit is alive, not if is complete.

Hans

On a sidenote, I find this repeated transfer of fuel and ammo to the
incomplete unit highly suspicious. Is the transfer code really supposed to
work with incomplete recepients, or is this another bug? Doesn't make sense
to me.

Hans Ronne

hronne@pp.sbbs.se