0

I made a concurrency class to test out threads. since I wanted to find out the best way to run threads at the same time.

I am surprised by my results:

test
test
Othertest
test
Othertest
test
test
test 

The results I expected were for the threads to come back randomly yet they seem to come back consistently in the same order! Does anyone know why? Does this mean that they are not running concurrently? How might I go about getting them to run at the same time?

Here is my code:

public class ThreadTest {
    public static void main(String args[]) throws InterruptedException
    {
        new Thread(new ThreadTest().test()).start();
        new Thread(new ThreadTest().test()).start();
        new Thread(new ThreadTest().otherTest()).start();
        new Thread(new ThreadTest().test()).start();
        new Thread(new ThreadTest().otherTest()).start();
        new Thread(new ThreadTest().test()).start();
        new Thread(new ThreadTest().test()).start();
        new Thread(new ThreadTest().test()).start();
    }

    public  Runnable test() throws InterruptedException{
        Thread.sleep((long) (Math.random()*1000));
        System.out.println("test");
        return null;
    }

    public  Runnable otherTest() throws InterruptedException{
        Thread.sleep((long) (Math.random()*1000));
        System.out.println("Othertest");
        return null;
    }

}
Prashant Kumar
  • 20,069
  • 14
  • 47
  • 63
kevinn2065
  • 325
  • 1
  • 3
  • 12

4 Answers4

2

The Thread constructor accepts a Runnable on which the Thread will eventually execute the run() method. Right now you aren't returning a Runnable object. you are returning null. So the execution you do in your test() and otherTest() methods is executed synchronously.

All your execution happens in one thread. This

new Thread(new ThreadTest().test()).start();

executes test(), sleeps for a second, prints "test" and returns null. The start() call doesn't do anything because the Runnable is null. This continues for each other call you do.

You need to put everything in your test() and otherTest() methods inside a Runnable#run() method. For example

new Thread(new Runnable() {
    public void run() {
         Thread.sleep((long) (Math.random()*1000));
         System.out.println("test");
    }
}).start();

Consider the source code of the run() method of Thread class, which is executed when start() is called

@Override
public void run() {
    if (target != null) {
        target.run();
    }
}

Where target is the Runnable reference you pass in the constructor. Obviously, if it is null, it won't do anything.

Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
1

Your Thread implementation is WRONG.

You should either implement Runnable and implement run() method or you should extend Thread class and override run() method.

What is happening is that your test() method or otherTest() is being called just as any method calls. And since you don't have any run() method your Thread.start() wont simply run anything.

Try changing your method like below.

public Runnable test() {
    return new Runnable() {
        @Override
        public void run() {
            try {
                Thread.sleep((long) (Math.random() * 1000));
                System.out.println("test");
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    };
}


public Runnable otherTest() {
    System.out.println("Othertest");
    return new Runnable() {

        @Override
        public void run() {
            try {
                Thread.sleep((long) (Math.random() * 1000));
                    System.out.println("Othertest");
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    };
}
Jayamohan
  • 12,734
  • 2
  • 27
  • 41
1

I think you might have better luck with this:

public class ThreadTest {
public static void main(String args[]) throws InterruptedException
{
    new Thread(test).start();
    new Thread(test).start();
    new Thread(otherTest).start();
    new Thread(test).start();
    new Thread(otherTest).start();
    new Thread(test).start();
    new Thread(test).start();
    new Thread(test).start();
}

public static Runnable test = new Runnable() {
    @Override
    public void run()  {
        try {
            Thread.sleep((long) (Math.random()*1000));
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("test");
    }
};

public static Runnable otherTest = new Runnable() {
    @Override
    public void run(){
        try {
            Thread.sleep((long) (Math.random()*1000));
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("Othertest");
    }
};

}

The idea is to pass an instance of Runnable as an argument to the Thread constructor. You are not really doing that, because test() and otherTest() are both returning null. The above code shows one way of running the threads in the way that I'm guessing you want. Other approaches are certainly possible.

kevinn2065
  • 325
  • 1
  • 3
  • 12
Ted Hopp
  • 232,168
  • 48
  • 399
  • 521
  • `Runnable`s are not allowed to throw `InterruptedException`. You actually have to catch those exceptions and interrupt the current thread when that happens. – C. K. Young Sep 02 '13 at 04:37
  • thanks I was just going to comment on that so i edited his comment – kevinn2065 Sep 02 '13 at 04:39
  • @ChrisJester-Young - Oops. You're right about that. I see that kevinn205 went ahead and corrected it. – Ted Hopp Sep 02 '13 at 04:44
  • This example Would not be thread safe either I presume unless I threw a synchonized or went back to initialing a new threadtest as well correct? – kevinn2065 Sep 02 '13 at 04:50
  • @kevinn205 - As long as you have multiple threads calling `System.out.println` without synchronization, there's the possibility of garbled output. (Although you can usually get away with it. See, e.g., [this discussion thread](http://stackoverflow.com/questions/9459657/synchronization-and-system-out-println).) Creating instances of `ThreadTest` won't help with that; you need synchronization. I don't know what you mean by "unless I threw a synchronized". – Ted Hopp Sep 02 '13 at 05:22
  • public static setItem(String item){String bob= item; system.out.println(item)} new thread.setItem(); new thread.setItem(); couldnt this code give the wrong value for bob because of the other thread? – kevinn2065 Sep 02 '13 at 06:00
  • @kevinn205 - That code is not legal Java, so it's hard to know what you're intending. If `bob` is a local variable (as it appears to be), then there's a separate `bob` for each thread and the threads won't interfere with one another. – Ted Hopp Sep 02 '13 at 07:14
1

You need to implement your test and otherTest methods as Runnable implementations. Like so:

private static class Test implements Runnable {
    @Override
    public void run() {
        try {
            Thread.sleep((long) (Math.random()*1000));
        } catch (InterruptedException e) {
            Thread.currentThread().interrupt();
            return;
        }
        System.out.println("test");
    }
}

private static class OtherTest implements Runnable {
    @Override
    public void run() {
        try {
            Thread.sleep((long) (Math.random()*1000));
        } catch (InterruptedException e) {
            Thread.currentThread().interrupt();
            return;
        }
        System.out.println("Othertest");
    }
}

public static void main(String args[]) {
    new Thread(new Test()).start();
    new Thread(new Test()).start();
    new Thread(new OtherTest()).start();
    new Thread(new Test()).start();
    new Thread(new OtherTest()).start();
    new Thread(new Test()).start();
    new Thread(new Test()).start();
    new Thread(new Test()).start();
}

You could, of course, try to reduce the duplication a little:

private enum Runnables implements Runnable {
    TEST {
        @Override
        public void run() {
            if (!sleep()) return;
            System.out.println("test");
        }
    },
    OTHER_TEST {
        @Override
        public void run() {
            if (!sleep()) return;
            System.out.println("Othertest");
        }
    };

    static boolean sleep() {
        try {
            Thread.sleep((long) (Math.random()*1000));
            return true;
        } catch (InterruptedException e) {
            Thread.currentThread().interrupt();
            return false;
        }
    }
}

public static void main(String args[]) {
    new Thread(Runnables.TEST).start();
    new Thread(Runnables.TEST).start();
    new Thread(Runnables.OTHER_TEST).start();
    new Thread(Runnables.TEST).start();
    new Thread(Runnables.OTHER_TEST).start();
    new Thread(Runnables.TEST).start();
    new Thread(Runnables.TEST).start();
    new Thread(Runnables.TEST).start();
}
C. K. Young
  • 219,335
  • 46
  • 382
  • 435
  • Thanks for the detailed response, Interesting it appears that you can wrap several runnables into a runnables enum and since you call them using implements it makes then a runnable when you access them using Runnables.test – kevinn2065 Sep 02 '13 at 04:44