Threads just stops working Java

Question

I'm programming a very simple simulation that is supposed to change field colors on w x h board. Each field on the board is a single Thread that changes it's color every random miliseconds. The thing is, after some time it just stops changing. Is there something wrong with my code, or is it optimalization thing, or something else? Is it some kind of deadlock? (Amount of threads does not change anything, just the time after it stops, debug board is 25x25)

I also found out that averageColor() may be causing the problem, becouse it works fine without it (f.e. with static color instead of average)

public void run() {
    while( true ) {
        try {
            int time;
            double q;
            int newcolor;
            synchronized( gen ) { // gen is a Random (one for the whole program)
                time = gen.nextInt( k ) + k/2; // k is a given parameter, 200 for debugging
                q = gen.nextDouble();
                newcolor = gen.nextInt( 256*256*256 );
            }
            synchronized( this ) { // posting rest of the class below
                wait( time ); // replacing with Thread.sleep( time ) does not fix the problem
                if( q < p ) { // with p probability it changes color to rando
                    me.setBackground( new Color( newcolor ) );
                }
                else { // with p-1 it pick an avarge
                    me.setBackground( averageColor() ); // averageColor takes an avarge of neighbour fields
                }
            }
            //Thread.yield(); // removed it as wrestang said, but it does not change anything
        } catch( InterruptedException e ) {
            e.printStackTrace();
        }
    }
}

Rest of the class:

public class Field extends Thread {

static Panel panel;
Label me;
Random gen;
int x, y, w, h;
int k;
double p;
Field[][] others;

enum Side {
    up,
    down,
    right,
    left
}

enum ColorPart {
    R,
    G,
    B
}

Field( Panel panel, Random gen, Field[][] others, int x, int y, int k, double p, int w, int h ) {
    this.gen = gen;
    this.others = others;
    this.x = x;
    this.y = y;
    this.k = k;
    this.p = p;
    this.w= w;
    this.h = h;

    me = new Label();
    me.setBackground( new Color( gen.nextInt( 256*256*256 ) ) );
    panel.add( me );

    setDaemon( true );
}

synchronized int getColor( ColorPart c ) {
    switch( c ) {
    case B:
        return me.getBackground().getBlue();
    case G:
        return me.getBackground().getGreen();
    case R:
        return me.getBackground().getRed();
    }
    return 0;
}

synchronized int getSideColor( Side side, ColorPart c ) {
    int a, b;
    switch( side ) {
    case down:
        b = y+1;
        while( b >= h ) b -= h;
        return others[x][b].getColor( c );
    case left:
        a = x-1;
        while( a < 0 ) a += w;
        return others[a][y].getColor( c );
    case right:
        a = x+1;
        while( a >= w ) a -= w;
        return others[a][y].getColor( c );
    case up:
        b = y-1;
        while( b < 0 ) b += h;
        return others[x][b].getColor( c );
    }
    return 0;
}

synchronized Color averageColor() {
    int r = 0;
    int g = 0;
    int b = 0;

    r += getSideColor( Side.up, ColorPart.R );
    r += getSideColor( Side.down, ColorPart.R );
    r += getSideColor( Side.right, ColorPart.R );
    r += getSideColor( Side.left, ColorPart.R );
    r /= 4;

    g += getSideColor( Side.up, ColorPart.G );
    g += getSideColor( Side.down, ColorPart.G );
    g += getSideColor( Side.right, ColorPart.G );
    g += getSideColor( Side.left, ColorPart.G );
    g /= 4;

    b += getSideColor( Side.up, ColorPart.B );
    b += getSideColor( Side.down, ColorPart.B );
    b += getSideColor( Side.right, ColorPart.B );
    b += getSideColor( Side.left, ColorPart.B );
    b /= 4;

    return new Color( r, g, b );
}

@Override
public void run() {
    while( true ) {
        try {
            int time;
            double q;
            int newcolor;
            synchronized( gen ) {
                time = gen.nextInt( k ) + k/2;
                q = gen.nextDouble();
                newcolor = gen.nextInt( 256*256*256 );
            }
            synchronized( this ) {
                wait( time );
                if( q < p ) {
                    me.setBackground( new Color( newcolor ) );
                }
                else {
                    me.setBackground( averageColor() );
                }
            }
        } catch( InterruptedException e ) {
            e.printStackTrace();
        }
    }
}

}

Created and started with:

public class SymPanel extends Panel {
private static final long serialVersionUID = 1L;

Random gen;
int w, h;
int k;
double p;
Field[][] fields;

SymPanel( int w, int h, int k, double p ) {
    this.w = w;
    this.h = h;
    this.k = k;
    this.p = p;

    setLayout( new GridLayout( h, w ) );

    gen = new Random();

    Field.panel = this;

    fields = new Field[w][h];
    for( int j = 0; j<h; j++ ) {
        for( int i = 0; i<w; i++ ) {
            fields[i][j] = new Field( this, gen, fields, i, j, k, p, w, h );
        }
    }
}

public synchronized void start() {
    for( int j = 0; j<h; j++ ) {
        for( int i = 0; i<w; i++ ) {
            fields[i][j].start();
        }
    }
}

}

In main:

public static void main( String[] args ) {
    frame = new Frame("Sym");
    frame.setBounds( 300, 100, 1024, 768 );
    frame.addWindowListener( new Sym() );

    sympanel = new SymPanel( 25, 25, 200, 0.3 );
    frame.add( sympanel );

    frame.setVisible( true );

    sympanel.start();
}

Is `gen` a `Random`? How is initialized? What is the value of `p`? — rgettman, May 02 '18 at 19:51
First question, why are you using Thread.yield? Look at the javadoc for that and see: https://stackoverflow.com/questions/6979796/what-are-the-main-uses-of-yield-and-how-does-it-differ-from-join-and-interr?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa as a quick note. Secondly, you don't ever show how this gets called, that could also be a problem. — wrestang, May 02 '18 at 19:52
First, you could generate thread dump of your application either in debugger or in console. Instead of block with `synchronized(this)` and `wait(time)` you could use `Thread.sleep(time)` — Ivan, May 02 '18 at 19:54
What is `me`? Is it a Swing component? If so [please read](https://docs.oracle.com/javase/tutorial/uiswing/concurrency/) What's `k`? If `k` is zero, this waits until interrupted (i.e. maybe forever). What's `this` in this context? — dhke, May 02 '18 at 19:56
Remove `synchronized( this )` block and instead of wait(time), use Thread.sleep(time). — Hemant Patel, May 02 '18 at 19:59
Re, "Each field on the board is a single Thread..." Probably not a good use for threads. I would make each field on the board an object in a priority queue where the "priority" of the object is the time when the field needs to be changed. Then a _single_ thread* could wait until it's time to change the field at the head of the queue, pop that object off, change it, compute a new random due-date for the object, and then put the object back into the queue and wait for the next one. — Solomon Slow, May 02 '18 at 20:04
*Except, I wouldn't even use _one_ thread for that. I'd use a `Timer` task. — Solomon Slow, May 02 '18 at 20:04
`setBackground()` should be called only on the main AWT thread. — Alexei Kaigorodov, May 02 '18 at 20:18
@jameslarge it's an exercise and I have to use threads like that. I added some more code, and commented most things that were asked, hope it's gonna help — Cansisti, May 02 '18 at 20:37
Try stripping away irrelevant code: https://stackoverflow.com/help/mcve — cmosher01, May 02 '18 at 20:50

score 0 · Answer 1 · answered May 02 '18 at 20:15

0

Just a couple of things:

A. When q in your code above exceeds (or matches) the value of p, the code will compute average color - which will be the same each time, assuming neighbouring fields haven't changed color.

B. An exception that causes the run loop to exit. I doubt this is the case, but you may want to catch all errors while you're debugging.

answered May 02 '18 at 20:15

Timir

1,395
8
16

The thing is it literally stops somewhere in `while( true )` loop, checked it by adding `println` in every loop start, it does not break out the loop as well (checked too). – Cansisti May 02 '18 at 20:40

score 0 · Answer 2 · answered May 02 '18 at 20:24

0

In general, Swing is not thread-safe. Any calls to Swing component methods must be done on the event dispatch thread. You can read about it in the Java Tutorials "The Event Dispatch Thread": https://docs.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html

answered May 02 '18 at 20:24

cmosher01

125
9

I'm using AWT tho, and I'm pretty new to Java as well – Cansisti May 02 '18 at 20:38

score 0 · Answer 3 · answered May 02 '18 at 21:10

0

Thanks to @cmosher01

Removing synchronized from getColor getSideColor and averageColor fiexed the problem, but I don't really get why. If anyone knows, I would appreciate some explanation why it may have caused the problem.

answered May 02 '18 at 21:10

Cansisti

153
7

score 0 · Accepted Answer · answered May 02 '18 at 21:14

What you get is a typical deadlock.

Consider the case of the two Fields A at x=0, y=0 and B at x=1, y=0.

At some point in time the Field A tries to calculate averageColor() - during this time it holds on lock on itself (since averageColor() is synchronized). Part of this calculation is calling getColor(..) on Field B, which is synchronized on Field B.

If at the same time Field B tries to calculate its averageColor(), Field B holds a lock on itself (so Field A cannot call getColor(..) on Field B). Part of this calculation is in turn to call getColor(..) on Field A - which blocks since Field A is holds a lock on itself and tries to call getColor(..) on Field B.

Threads just stops working Java

4 Answers4