PDA

View Full Version : Java Thread Concurency


Flash Gordon
01-06-2010, 08:05 AM
This probably isn't the best place to ask, but I'll see what happens.

Scenario:
I'm trying to make 2 threads have a reference to 1 object and loop iterate over that object and print out it's values.

Problem:
The thread falls asleep after the check for hasNext() and when it executes next() there really is no next because the other thread grabbed it. Also, I'd expect to see the values printed in order.

Java:

import java.util.ArrayList;
import java.util.Iterator;


public class TDD
{


public static void main(String[] args)
{
ArrayList<Character> list = new ArrayList<Character>();

for (int j=0; j<20; j++)
{
Character ch = new Character('\u0041');
for (int i=0; i<26; i++)
list.add(new Character(ch++));
}

Runnable run = new IterRunnable(list.iterator());
Thread one = new Thread(run);
Thread two = new Thread(run);
one.setName("One");
two.setName("Two");
one.start();
two.start();
}
}

Java:

import java.util.Iterator;


public class IterRunnable implements Runnable
{
private Iterator iter;

public IterRunnable(Iterator iter)
{
this.iter = iter;
}

@Override
public void run()
{
System.out.println( "RUNNING! " + Thread.currentThread().getName() );
boolean loop = true;
while(loop)
{
// synchronized (this)
// {
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
// }
}
}
}


You can see I've tried to synchronize the inner part of the loop but it seems to lock it so only a single thread will iterate over the object.

Any help?

ASWC
01-06-2010, 01:29 PM
If I remember correctly it's most likely the java stackflow that's doing that. Even though you have two threads, what's running is determined by the Java stackflow via a system of priority. So basically the Java stackflow runs the next operation in the stack and then the next and so on.

Eric Drutel
01-06-2010, 01:57 PM
Pass to IterRunnable a reference to the list itself, not an iterator. Also, if you concurrently modify the list you should synchronize on list. Iterators returned by iterator and listIterator methods are fail-fast (if list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException)

http://java.sun.com/j2se/1.5.0/docs/api/java/util/ArrayList.html

Flash Gordon
01-07-2010, 12:41 AM
i need a reference to the iterator because list.iterator() returns a new iterator everytime. I'm trying share the iterator and not the list.

bowljoman
01-07-2010, 12:54 AM
If you must do this, then wrap the list in another class with syncrhonized get methods.

Or.... what I would do is use an Executor instead of runing the subclassed Thread.

Or... clone the list

Or... there are a hand full of other ways.

Basically the code that touches the list will need a Lock so that the list is only touched by one thread at a time.

You were pretty close. Use an object for the lock that is visible to everyone.




synchronized (SOME_STATIC_OBJECT)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}

bowljoman
01-07-2010, 12:56 AM
You have a problem though.

Only one thread will actually have elements.

bowljoman
01-07-2010, 12:58 AM
I would wrap the list in an object with synchronized getNextElelment, and make the Thread pull a single element at a time.

This way you will have the list available to all threads.

If the method returns null, kill the thread.

bowljoman
01-07-2010, 01:00 AM
Pass a reference to the list-containing-object and define the interface for the thread to call for the next item.

You will be able to spawn a thread for each element if you want to that way

Flash Gordon
01-07-2010, 03:06 AM
You have a problem though.
Only one thread will actually have elements. Can you explain further what you mean?


You were pretty close. Use an object for the lock that is visible to everyone.


synchronized (SOME_STATIC_OBJECT)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}

so would then synchronized (iter) {} work? It doesn't seem to.

I'm fairly new to Java Threads so let me explain what I'm trying to do by showing the desired output.

One A
One B
Two C
One D
Two E
Two F
......

So that the iteration through the object is shared by several threads (more than 2 eventually). Maybe I'm going about it wrong, but that's the desired output.

bowljoman
01-07-2010, 06:57 AM
The lock is within each of your classes, and the first one to get there wins and consumes them all.




synchronized (SOME_STATIC_OBJECT)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " );
}
}




You will need the pattern I mentioned.

Make an object that wraps the iteration and provide a synchronized get.

Something like this



public class wrapper {
private Iterator iter;
public wrapper(Iterator iter){
this.iter=iter;
}
public synchronized Object getNext(){
if(iter.hasNext())
return iter.next();
else return null;
}
}


public class IterRunnable implements Runnable
{


private wrapper iter;

public IterRunnable(wrapper iter)
{
this.iter = iter;
}

@Override
public void run()
{
System.out.println( "RUNNING! " + Thread.currentThread().getName() );
boolean loop = true;
while(loop)
{

Object thing = iter.getNext();
if (thing !=null)
{
System.out.println( Thread.currentThread().getName() + " " );
}
else loop=false;

}
}
}

Eric Drutel
01-07-2010, 06:59 AM
I think Bowljoman's idea is good.
You could do something like:


public class MyList extends ArrayList {
private int myCounter = 0;

public Object getNextElement() {
return get(myCounter);
myCounter++;
}

public int getMyCounter() { return myCounter;}
}


an then in run


while (true) {
synchronized(myList) {
if (myList.getMyCounter() < list.size())
myList.getNextElement();
else break;
}
}


Btw.. I did not test the code, but it should have output you mentioned. Let me know if it works.
Regards.

Flash Gordon
01-07-2010, 07:17 AM
thanks guys. I'll try it tomorrow night and see what happens.
cheers

Flash Gordon
01-10-2010, 02:31 AM
The lock is within each of your classes, and the first one to get there wins and consumes them all.
got ya, because it's locking an instance and I'm sharing and instance of IterRunnable. So then if I don't share the Runnable instance wrappers it should work....

Iterator<Character> iter = list.iterator();
//Runnable run = new IterRunnable(list.iterator());
Thread one = new Thread( new IterRunnable(iter) );
Thread two = new Thread( new IterRunnable(iter) );
one.setName("One");
two.setName("Two");
one.start();
two.start();


@Override
public void run()
{
System.out.println( "RUNNING! " + Thread.currentThread().getName() );
boolean loop = true;
while(loop)
{
synchronized (this)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}
}
}

It runs almost as expected but this is my output.

One Z
One A
One B
RUNNING! Two
Two D
Two E
Two F
Two G
Two H
Two I
Two J
One C
One L
One M

So now the threads are taking turns. But the order is unexpected and on occassion, it still goes over and tries to get the next() when it shouldn't.

I apologize for not pasting your code directly, but I'm trying to learn a bit more than a copy and paste. And going by your answer that I quoted above, I would expect to see the correct results. Ideas?

thanks.

bowljoman
01-10-2010, 09:47 PM
1. What makes you think that they would take turns? They wont, and don't count on them to how you have written it. There is no 'take-turn' mechanism in yours or my code. java really is multi threaded.

2. Is there a reason to lock the object running through an iterator ? No. Again, you need to lock access to the iterators 'Next' but you only lock the handlers. Nothing is accessing those objects so the locks do nothing.

Your two objects need to acquire the same lock object.

Even with my code you cant expect that they will alternate. They wont, however, mine does synchronize the threads by encapsulating the 'next' call in a synchronized function.

bowljoman
01-10-2010, 09:49 PM
synchronized means only one thread can call that method at a time, not that threads will politely take alternating turns. I think you knew that , but thought I would make sure.:D

Flash Gordon
01-10-2010, 11:14 PM
1. What makes you think that they would take turns? They wont, and don't count on them to how you have written it. There is no 'take-turn' mechanism in yours or my code. java really is multi threaded. Ok true, and yeah I do know that. Sure there is no guarantee they will, but when one does sleep the other will become active. "Taking turns"...I misspoke. I mean to talk more in terms of synchronization and not alternating. Pardon me.

Your two objects need to acquire the same lock object. The same lock? Really, not that each should have it's own lock. Well....that's how I had it originally and it didn't work as expected. It didn't "take turns." Yes there is no guarantee it should, but it often should.


synchronized (this)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}

Does this mean that the object/thread will not fall asleep between the "synchronized (this)"? So I would expect it to fall asleep before or after and then the next thread would pick it up. The keyword synchronized means get a lock for the object and release it once done with synchronized part? If so, they why doesn't synchronizing the wrapper do any good and no one should be touching the iterator unless it is their "turn"?

Once again....forgive me as I'm trying to understand the concept and not just a solution to a single problem instance. I guess I'm still not getting it.....mabye I'll reread a couple of sections on Threads and come back and ask some specific questions. Thank you for your patience and help.
Cheers

bowljoman
01-11-2010, 03:38 AM
How many of these do you have?


public void run()
{
System.out.println( "RUNNING! " + Thread.currentThread().getName() );
boolean loop = true;
while(loop)
{
synchronized (this)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}
}
}



Each has their own lock, and nobody is going to go to sleep.

How the heck ar you goin to make one sleep if they are not trying to acquire the same lock?

You are locking on 'this' within each individual object. It has no effect between the two instance or even within it self. Nothing is going to sleep.

bowljoman
01-11-2010, 03:40 AM
synchronized (THE_SAME_REFERENCED_OBJECT)
{

}

Flash Gordon
01-11-2010, 04:03 AM
Each has their own lock, and nobody is going to go to sleep.

How the heck ar you goin to make one sleep if they are not trying to acquire the same lock?

You are locking on 'this' within each individual object. It has no effect between the two instance or even within it self. Nothing is going to sleep.
perfect, that makes sense. now let me study the examples and come back to this again.

Summary, each Runnable (or rather synchronization) must fight for the same lock! (correct me if I'm wrong).

bowljoman
01-11-2010, 04:14 AM
yup

Flash Gordon
01-11-2010, 04:50 AM
ok...so how about this:
java:

import java.util.ArrayList;
import java.util.Iterator;

public class TDD
{
public static void main(String[] args)
{
ArrayList<Character> list = new ArrayList<Character>();

for (int j=0; j<20; j++)
{
Character ch = new Character('\u0041');
for (int i=0; i<26; i++)
list.add(new Character(ch++));
}

Iterator<Character> iter = list.iterator();
Thread one = new Thread( new IterRunnable(iter) );
Thread two = new Thread( new IterRunnable(iter) );
one.setName("One");
two.setName("Two");
one.start();
two.start();
}
}

java:

import java.util.Iterator;


public class IterRunnable implements Runnable
{
private Iterator iter;

public IterRunnable(Iterator iter)
{
this.iter = iter;
}

@Override
public void run()
{
System.out.println( "RUNNING! " + Thread.currentThread().getName() );
boolean loop = true;
while(loop)
{
synchronized (iter)
{
loop = iter.hasNext();
if (loop)
{
System.out.println( Thread.currentThread().getName() + " " + iter.next());
}
}
}
}
}

It works exactly as expected. I have 2 Runnable both fighting for 1 lock in the iterator. You provided a different solution earlier. In terms of syncro or even objects, does yours have an advantage over this one that I should be aware of? Or perhaps even this one is fooling me into thinking it is working correctly???

Thank you for your time and patience. You've helped me to learn an fundamental which is much more important than just solving an issue.

bowljoman
01-11-2010, 05:03 AM
looks the same to me, probably be fine like that.

ASWC
01-11-2010, 05:12 AM
results look good at the start but then it gets messy.

Flash Gordon
01-11-2010, 05:19 AM
results look good at the start but then it gets messy.hmm....not for me. You sure?

ASWC
01-11-2010, 05:25 AM
here's some of what I get when I run your last code:
Two M
One E
Two N
One F
Two O
One G
Two P
One H
Two Q

bowljoman
01-11-2010, 05:52 AM
looks great.

No collisions. and no error from concurent 'hasNext == true'

Eric Drutel
01-11-2010, 05:56 AM
Try like this:


public class AA {

public static void main(String[] args)
{
MyList<Character> list = new MyList<Character>();

for (int j=0; j<20; j++)
{
Character ch = new Character('\u0041');
for (int i=0; i<26; i++)
list.add(new Character(ch++));
}
Thread one = new Thread(new MyRunnable(list));
Thread two = new Thread(new MyRunnable(list));
one.setName("One");
two.setName("Two");
one.start();
two.start();
}

}

class MyList<T> extends ArrayList<T> {
private int cont = -1;

public int getNextCont() {
cont++;
return cont;
}

public boolean hasMoreElem() {
return cont < this.size();
}
}

class MyRunnable<T> implements Runnable
{
private MyList<T> list;

public MyRunnable(MyList<T> list)
{
this.list = list;
}

@Override
public void run()
{
while (true) {
synchronized (list) {
int cont = list.getNextCont();
if (cont < list.size()) {
System.out.println(Thread.currentThread().getName( ) + " - " + list.get(cont));
} else {
break;
}
}
}
}
}

Flash Gordon
01-11-2010, 05:56 AM
I'm pretty happy with the results of this thread. I'm going to consider it resolved, but I won't close it. thanks again everyone!

ASWC
01-11-2010, 07:26 AM
well I thought you wanted to get that:
RUNNING! One
One A
RUNNING! Two
Two B
One C
Two D
One E
Two F
One G
Two H
One I
Two J
One K
Two L
One M
Two N
One O
Two P
One Q
Two R
One S
Two T
One U
Two V
One W
Two X
One Y
Two Z
Anyway that's the way I got it -> alternating thread and iteration, useful.

Flash Gordon
01-11-2010, 07:45 AM
on my i get a straight up list, but only about 2 alterations...which is just fine since I can't control it (maybe only influcence it a bit). My next tackel is I'm actually looking for a result from 1 of the 2 threads and when I get that result I need to break both threads and save the output. We'll see what I can do with that since Runnable doesn't exactly return any objects....

ASWC
01-11-2010, 07:52 AM
Take a look also at the Collections class which has a synchronizedList static method especially made for multithread synchronizing on arraylists.

Flash Gordon
01-11-2010, 08:22 AM
nice. thank you for the heads up!

ASWC
01-11-2010, 09:17 AM
We'll see what I can do with that since Runnable doesn't exactly return any objects....Not sure what you mean by that. You can create any public method in your Runnable class and return what you want.

Eric Drutel
01-11-2010, 01:56 PM
on my i get a straight up list, but only about 2 alterations...which is just fine since I can't control it (maybe only influcence it a bit). My next tackel is I'm actually looking for a result from 1 of the 2 threads and when I get that result I need to break both threads and save the output. We'll see what I can do with that since Runnable doesn't exactly return any objects....

There is also an Callable interface, that is similar with Runnable, but returns an object. Take a look at SchedulerExecutor example from here. You can replace the Runnable with a Callable and use ScheduledFuture to retrive results: http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ScheduledExecutorService.html