thread start/stop does not work correct on android

Question

I need some help with starting/stopping my monsters.
We generate like this:

for (int i = 0; i < 20; i++) {
    boolean notFree = true;
    int x = 0, y = 0;
    // while not free change
    // not determinstisch setting!
    while (notFree) {
        x = (int) (Math.random() * Config.XSIZE);
        y = (int) (Math.random() * Config.YSIZE);
        if (map.getMapPosition(x, y) == Config.EMPTYPOSITION) {
            notFree = false;
        }
    }
    switch ((int) Math.floor(Math.random() * 2)) {
    case 0:
        monsterVerktor.add(new Monster(context, x, y, this.map, 1,
                this.charac));
        break;
    case 1:
        monsterVerktor.add(new DeathReaper(context, x, y, this.map, 1,
                this.charac));
        break;
    default:
        break;
    }
}

And I stop them like this: (Start is same just with Startmoving...)

public void stopAllMonsters() {
    for (Monster monster : monsterVerktor) {
        monster.getControl().stopMovingThread();
    }
 }

The thread.stopMovingThread works like this:

public void stopMovingThread() {
    this.monster.setAlife(false);
    running = false;
    moveDone = true;
    boolean retry = true;
    while (retry) {
        try {
            thread.join();
            retry = false;
        } catch (InterruptedException e) {
            // try again shutting down the thread
        }
    }
}

To the run():

public void startMovementThread() {
    running = true;
    thread = new Thread() {

        @Override
        public void run() {
            while (running) {
                Log.d(TAG, "run Thread");
                // get a little randomness in the movement ;)
                try {
                    // up to 0.5s sleeping till next moving
                    sleep(new Random().nextInt(Config.RANDOMSLEEP));
                } catch (InterruptedException e1) {}
                while (monster.isAlife()) {// check if alife
                    moveDone = false; // resett movement Done
                    noMove = false;
                    // Charakter in near if one is in near he
                    // will move to it.
                    if (!checkCharInAggroRange()) {
                        noMove = rndMove(); // if we have no move
                    }
                    while (!moveDone) {
                        timeBegin = System.currentTimeMillis();
                        // if hes not done with the move
                        if (monster.moveToX == monster.positionX
                            && monster.moveToY == monster.positionY) {
                            if (noMove) {
                                try {// sleep because no movement with a bit
                                        // randomness
                                    sleep(Config.MONSTERMOVINGTIME
                                        + (new Random()
                                                .nextInt(Config.RANDOMSLEEP)));
                                } catch (InterruptedException e) {
                                    Log.d(TAG,
                                        "An error occured during sleep because"
                                            + " of no Animatino");
                                }
                            }
                            // false after sleep
                            moveDone = false;
                            break; // already at the right position!
                            // movetoX is left of position
                        } else if (monster.moveToX > monster.positionX) {
                            monster.positionX++;
                            // movetoX is left of it
                        } else if (monster.moveToX < monster.positionX) {
                            monster.positionX--;
                            // movetoY is left of position
                        } else if (monster.moveToY > monster.positionY) {
                            monster.positionY++;
                            // movetoY is left of position
                        } else if (monster.moveToY < monster.positionY) {
                            monster.positionY--;
                        }
                        // sleep if hes moving to fast!
                        timeDiff = System.currentTimeMillis() - timeBegin;
                        sleepTimer = (int) (Config.MONSTERTIMING - timeDiff);
                        if (sleepTimer > 0) { // if >0 we are fast enough
                            // and can sleep a bit ;)
                            try {
                                sleep(sleepTimer);
                            } catch (InterruptedException e) {
                                Log.d(TAG, "Gameloop thread cant sleep");
                            }
                        }
                    }
                }
                try {
                    sleep(Config.RESPAWNTIMER);
                } catch (InterruptedException e) {
                    Log.d(TAG, "Monsterthread cant sleep");
                }
                // respawn it after a sleep :)
                respawn();
            }
        }
    };
    thread.start();
}

Android stops working if we want to stop them and start them like this, and I dont get it why. We also stop/start our render like this. (Well it's a runable and this isn't)

Monster isAlife and etc:

public class Monster extends Drawable {

    private static final String TAG = Monster.class.getSimpleName();
    private int fullLife; // full life
    private int curLife; // current life
    public Context context;
    private Character charac;
    // ..
    // Basic monsterstuff
    private boolean alife, moveDone;
    private int level;
    public Status status;
    private MonsterControl control;

    // ..
    public Monster(Context context, int mapPosX, int mapPosY, Map map,
            int level, Character cha) {
        this.context = context;
        this.map = map;
        this.setCharac(cha);
        this.mapPosition[0] = mapPosX;
        this.mapPosition[1] = mapPosY;
        this.status = Status.IDLE;
        // example for full life calculation
        this.fullLife = level * 100 + ((int) (Math.random() * 10 * level)); // Examples
        this.curLife = this.fullLife;
        this.map.setMapPosition(mapPosX, mapPosY, Config.MONSTERSTATE);
        // set monster position
        // ..
        // load the sprite bitmap
        // ...
        // Change this later!
        alife = true;
        Log.d(TAG, "Monster created");
        // Starting the Controler
        control = new MonsterControl(this, this.charac);
        control.startMovementThread();
        Log.d(TAG, "Monster start moving");
        // exemplarisch cut of the sprite
        this.monsterPicAnimation();
    }
}

here the getter/setter for aLife

public boolean isAlife() {
    return alife;
}

public void setAlife(boolean alife) {
    this.alife = alife;
}

Answer 1

I solved it myself with and "good" idea. The problem was, that i let them sleep at some points. If they sleep it cant stop/join the thread and even dont watch if its "running". So i solved it with a small methode that looks like this:

private void breakableSleep(int i, int intervall){
    int x = 0;
    while(running && x <= i){
        try {
            this.thread.sleep(intervall);
        } catch (InterruptedException e) {
            Log.d(TAG, "Monsterthread cant sleep");
        }
        x++;
    }
}

it can be stoped at every intervall. Sure it will cost some Battery but did not finde any other way to stop the Monsters in a "fast" way because some sleep for around 3seconds if they died (respawning) I also rewrote the whole startMovementThread because it did not look well in my opinion.

Best Regards

Answer 2

Apart from the logic of your code which I haven't looked at in details, there is a lack of synchronization around shared variables. In particular, as it is, your code provides no guarantee that your thread (in startMovementThread) will ever be able to observe the changes made to your flags from the main thread.

A good first step would be to make those variables volatile:

private volatile boolean alife, moveDone;

Note however that there might be other issues (depending on your requirements). For example, when those statements are executed in startMovementThread:

if (monster.moveToX == monster.positionX ...)
monster.positionX++;

any changes made in that thread might not be visible from the main thread and vice versa. If that can be an issue, you need to synchronize access to those variables too (and making them volatile will not be enough because, for example, monster.positionX++ is not atomic).

Answer 3

First off, your comment of "Android stops working", Are you implying a crash? Got a LogCat trace?

Other than that is this analysis correct?

1.Create a monster.

running = true;
isAlife = true; //or whatever monster.isAlife() checks

2.Thread starts.

while (running) {
  ...
  while (monster.isAlife()) {
    ...
  }
}

3.You try to stop the thread by (1) setting running to false, and (2) joining on the thread?

---

Assuming that is all true. The monster thread still runs until something kicks it from the nested while(monster.isAlife()) loop. Once that ends, the while(running) will evaluate false and the thread should terminate, moving on to the next monster in your collection. Without this ever terminating you'll be waiting on each monster to die while locking your main thread with join().