CODEWITHSUNDEEP
javamultithreading
Vojtěch Kaiser
Vojtěch Kaiser

Reputation: 568

Main thread keeps waiting on new threads

i'm trying to write some "game engine" for my projects and I'm facing a problem with threads. When I create a Thread (LoadThread) in main flow, it keeps waiting until Run(); in LoadThread ends.

//loading thread
public class LoadThread implements Runnable{
    private boolean running = false;
    private Thread loader = null;

    public LoadThread(/*init data structures*/){
        loader = new Thread(this);
    }

    public void start(){
        running = true;
        run();
    }

    synchronized public void run() {
        System.out.println(" loading started ");
        while(running){            
            //do some loading, when done, running = false
        }
        System.out.println(" loading done ");
    }
}

//holds data, starts loading
public class SourceGod {
    private LoadThread img_loader;
    public void startLoading(){
        img_loader = new LoadThread(/* some data structures */);
        img_loader.start();
    }
}

//runs the game
public class Game extends GameThread implements ActionListener{
    private SourceGod sources;
    public Game(Window full_screen){
        sources = new SourceGod(/* some data structures */);
        System.out.println("before");
        sources.startLoading();
        System.out.println("after");
    }
}

//own thread to refresh
abstract public class GameThread extends JPanel implements Runnable{
    //anything from there is not called before "after"
}

output

before
loading started
//some loaded data report, takes about 2-3s
loading done
after

Any help will be appreciated. ( little more of code http://paste.pocoo.org/show/orCfn9a8yOeEQHiUrgjG/ ) Thanks, Vojtěch

Upvotes: 0

Views: 253

Answers (2)

Gray
Gray

Reputation: 116858

Your problem is that you are creating a Thread object in your constructor but when you call start() you are not starting the thread but running the run() method in the same thread. This looks to be confusion around extending Thread versus implementing Runnable.

What I think you should do instead is something like:

// this should be of type Thread not LoadThread
private Thread img_loader;
...
// don't create the loader thread inside of LoadThread
img_loader = new Thread(new LoadThread(/* some data structures */));
img_loader.start();

LoadThread is the class of type Runnable that the thread will be executing. With your current code, when it calls:

img_loader.start();

It isn't actually starting a new Thread, it is just calling the LoadThread.start() method which calls run() in the same thread:

public void start(){
    running = true;
    run();
}

Edit:

In looking more closely at your posted code in the link you provided, you are creating your Thread object inside of the LoadThread constructor. This is not a good pattern:

public LoadThread(/*init data structures*/) {
    // not recommended IMO
    loader = new Thread(this);
}

So again, when you are calling LoadThread.start() you aren't starting the thread but calling run() in the same thread. I would use the new Thread(new LoadThread()) pattern instead. If you are stuck on wrapping the Thread inside of LoadThread then you should change LoadThread.start() to be:

// this should be removed, you want to call Thread.start() instead
public void start(){
    running = true;
    // this will start the internal thread which will call run()
    loader.start();
}

Btw, if you want to set running to be false in another thread then it should be marked as volatile.

Upvotes: 5

Snicolas
Snicolas

Reputation: 38168

You have several problems in the way you use the Runnable interface :

  • a runnable is way to wrap code so that it can be launched in a different thread. You should override the run method. And not call it directly, never.
  • to use a runnable, wrap into a thread and run the thread start method. As such you should not override the start method of a thread, or always call super.start().
  • providing a start method to a runnable is just messy. The start method of the wrapping thread will call the run method of your runnable in the new thread (the wrapper one). That's it. So if you have a start method in your runnable, it will never be called by the wrapping thread.
  • never call the run method of either a thread or a runnable. You would miss the point as the method would be executed in the current thread, and not in a new one.
  • don't synchronize a run method, it is intended to be used only by one thread already.

Here is what you should do in your case : 

//loading thread
public class LoadThread implements Runnable{
    /** Wether or not the thread is running. */
    private boolean running = false;
    /** Wrapper thread. */
    private Thread loader = null;

    public LoadThread(/*init data structures*/){
        loader = new Thread(this);
    }

    public void start(){
        running = true;
        loader.start();
    }

    @Override
    public void run() {
        System.out.println(" loading started ");
        while(running){            
            //do some loading, when done, running = false
        }
        System.out.println(" loading done ");
        running = false;
    }
}

As you see, If you want a start method, just call the wrapper thread start method.

Also, if you really need to call start, consider not using a runnable but a thread directly : 

 //loading thread2
 public class LoadThread extends Thread{
    /** Wether or not the thread is running. */
    private boolean running = false;

    @Override
    public void run() {
        running = true;
        System.out.println(" loading started ");
        while(running){            
            //do some loading, when done, running = false
        }
        System.out.println(" loading done ");
        running = false;
    }
}

That is much simpler.

Upvotes: 0

Related Questions