🔒 Locked

Java Thread problem, occasionaly hangs

Started by johnnyBravo Sep 14, 2007 at 8:26 AM 3 replies 1.9k views

Original Post

Sep 14, 2007 at 8:26 AM

Sometimes in my multithreaded program after starting 2 threads it hangs, although the close button is clickable, but the area inside the window is frozen, and also in Eclipse the little red stop running button is no longer clickable. I'm sure its because the way I'm using the threads. Heres how it goes: -load xml from file into document -start thread to listen for connections --on connection start another thread and receive input there ---add data to xml document ---save document to xml file I put 'volatile' on the variables that are used by multipe threads. eg private volatile Document document; private volatile LogPane logPane = new LogPane(); //just a JPanel with a textarea, and scrollpane And 'synchronized' on the functions that are also used by multiple threads. eg private synchronized void saveDocument(); private synchronized void addSellerToDocument(Node node); private synchronized void onConnection(final Socket socket); Am i handling the accessing of variables and functions from different threads wrongly? thanks edit: Also the series of events is: the first program waits for a connection on a separate thread another program connects, sends data and disconnects the first program creates a another thread to handle the connection, read the sent data and save it to file.

Antheus

Sep 14, 2007 at 8:32 AM

private volatile Document document;private volatile LogPane logPane = new LogPane();

These are redundant. The assignment to them isn't problematic, and unless that assignment is synchronized, volatile doesn't prevent multiple threads from trashing the value.

volatile only makes sense for assignments, or in case of values, for primitive types.

private synchronized void saveDocument();private synchronized void addSellerToDocument(Node node);private synchronized void onConnection(final Socket socket);

This could be whatever. Synchronized isn't something you just toss around. What are the contested resources here? What do you need to make sure is thread-safe?

If your threads access same document object, then it's better to synchronize on that explicitly:

...synchronized(document) {  document.foo();}...

Of course, the document instance must not change.

As for why the app doesn't close... Could be anything.

johnnyBravo

Sep 14, 2007 at 8:45 AM

Quote:
Original post by Antheus

*** Source Snippet Removed ***

This could be whatever. Synchronized isn't something you just toss around. What are the contested resources here? What do you need to make sure is thread-safe?

the 'document' and the 'logPane' is what I'm trying to protect from being written to at the same time.

And the 'saveDocument()' I want only to be called once at a time, so it doesn't try to save to the file while already doing that.

Quote:
Original post by Antheus
If your threads access same document object, then it's better to synchronize on that explicitly:
*** Source Snippet Removed ***

Of course, the document instance must not change.

So I just wrap my code inside each of the functions with that?

Quote:
Original post by Antheus
As for why the app doesn't close... Could be anything.

I realised its not that it won't close, but that it has hanged, the window controls still respond eg resize, close, maximise buttons etc, though the inner part of the window does not respond.

Antheus

Sep 14, 2007 at 9:13 AM

My usual approach would be something like this:

class SocketReader implements Runnable{  public SocketReader( Socket s, Document d )  {    socket = s;    document = d;    // attack to socket stream  }  public void run()  {    while (running) {      // read from socket      synchronized( document ) {        //--------- single-threaded --------------        document.// add stuff to document        document.// do more stuff        document.save();        //--------- end single-threaded ----------      }    }  }  private Socket socket;  private Document document;}

As long as you don't access "document" from anywhere else, this guarantees you thread-safety.

johnnyBravo

Sep 14, 2007 at 10:13 AM

Quote:
Original post by Antheus
As long as you don't access "document" from anywhere else, this guarantees you thread-safety.

Do you mean anywhere else as in another thread?

I think the reason it was hanging was because I was using 'volatile' in conjunction with 'synchronized', which for some reason is ocasionally bad.

Also is it ok to use synchronized on the file saving method, to make sure it only run once at a time?

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.