aspose file tools*
The moose likes Threads and Synchronization and the fly likes gUI freezes - Deadlock problem? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "gUI freezes - Deadlock problem?" Watch "gUI freezes - Deadlock problem?" New topic
Author

gUI freezes - Deadlock problem?

Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45
I have an issue with a server - client program that causes deadlocks as I believe. The server class
creates a cleintHandler class and starts a thread there. This clientHandler class
creates MathNumbers that is sent away to the client. The number shows up in the client,(that also has an inner class with a Thread running) that
has a gui, and you're supposed to answer the number, i e putting in the number in the textfield and
clicking on the SendAway Button. The number is sent to the server that calculates the number and
sends back a true or false
The client recieves this boolean and displays if the answer was correct or false. So, this cycle goes
on like that, and you can start up as many clients as you want. The problem is that the client gui
keeps on freezing all the time. The server can send away 2, 3, 4 or 5 numbers but all of a sudden the
window freezes.
I understand that there is this method invokeLater in SwingUtilities but I don't really know how to
apply it here. I don't have Thread implemented in my gui class. The code at the send answer button
looks like this:



Someone said to me that he didn't believe that this was a invokeLater issue. But I don't know, anyone
that has an idea how to implement invokeLater here or if I at all should implement it?

The gui has the EDT and the main thread.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

You really do need to be pushing the work out of the GUI thread into another thread - not using invoke later, but using a background thread (like SwingWorker). If you do not know how to do so, read this: http://docs.oracle.com/javase/tutorial/uiswing/concurrency/

That said, that will make your app more responsive, and will keep your application from freezing, but may not fix the problem - instead of freezing the client may just never get a response. To fix the problem you need to know where the hangup is. It could be in the server, it could be in the client, but it is impossible to say with the code you posted. Use a debugger on both the client and server, or use print statements on both sides to see where things get when the application hangs. Once you know where the problem is, fixing it gets much easier.


Steve
K. Tsang
Bartender

Joined: Sep 13, 2007
Posts: 1966
    
    7

When the client send numbers to the server, is it asynchronous or synchronous (client waits till server return)?

The send number to server feature should be done in another thread. When the thread returns something from the server, the GUI thread gets notified and inform the user.


K. Tsang JavaRanch SCJP5 SCJD/OCM-JD OCPJP7
Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45

Thanks for answering,

When the client send numbers to the server, is it asynchronous or synchronous (client waits till server return)?


It is synchronous, i e it waits till server return.

This is how I've done it. And it is supposed to be correct.
MathClient class:


The send number to server feature should be done in another thread.

MathClient class has it's own Thread. The server has another Thread. Or is this what you ment?



or use print statements on both sides to see where things get when the application hangs


I will start to look into this and do some System.out.println(); on various places and see if I can find anything.



Thanks for answering.// Anders

K. Tsang
Bartender

Joined: Sep 13, 2007
Posts: 1966
    
    7

Since you mentioned synchronous, then you should also check whether it is the client (GUI) waiting for the server return that "tends" to freeze (maybe server side) or nothing to do with the server code.
Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45

Since you mentioned synchronous, then you should also check whether it is the client (GUI) waiting for the server return that "tends" to freeze (maybe server side) or nothing to do with the server code.


How do I check that?


I have been changing around the code a bit now but then I do get other problems. I have a server class, clienthandler class and a client class(+gui for client).

Server:
Here I have the Serversocket and I start up the client handlers with the code:


All the streaming at the server side is taken care of in the client handler class, i e

The server crates a new MathNumber and sends this (object) to the client

Then the client recieves the object, presents it to the user and waits for the user to put in the answer.

The user puts in the answer which is stored in the object and the client sends it back to the server

The server recieves the mathnumber(that now contains the answer from the user )


The server calculates the answer and sees if it was false or true and sends that back to the cleint

The client recieves the result(a boolean) and presents it in a suitable way for the user.


My question here is; on the server side , should it all be handled in the client handler class or should part of this be done in the server, (the first sending of a new mathnumber)?

//Anders
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

It is still unclear to me. Can you provide the server code?

Have you done the print statements as you have been asked? Put a System.out.println() after EVERY line on both the server and the client. Put key information (such as line of code, value of key variables, state of sockets, whatever seems important or informative as a result of the previous line of code or necessary for the next line of code). Then look through the output to see what you get.
Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45
I have done the print statements and it helps out a bit but not the whole way. (what do you mean with state of sockets?)

Here is the server class:


The client handler:


Are there to many streams in and out of the client handler, or is it not possible to build it that way, i e that I put the first stream, creating the mathnumber, in the server not the clienthandler and sending it from the server to the client instead of sending it from the clienthandler?

Anders
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

Anders Kviback wrote:I have done the print statements and it helps out a bit but not the whole way. (what do you mean with state of sockets?)

What do you mean by a bit but not the whole way? Which line of code did the Server get to? When you got to the out.writeBoolean(answer) line, what was the value of answer, what was the value or serverCalc, what was the value of kSvar? A Socket has some methods which will tell you if it is bound, connected, closed, or if its streams are shut down. Look at the API, then use those results in the println statements.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

A few comments on the code:

1) Your code allows the socket and its input streams and output streams to stay connected and open even if an exception occurs and you abandon the Thread. This is horrible, and will lead to resource leaks.
2) Your code also allows exception to occur, get reported, then the application to continue as if nothing was wrong. If you didn't fix the problem, then the resources should be cleaned up and the thread stopped, and probably some nasty and informative information posted to the error stream about the problem. But you should not let execution to continue.
3) This code:

Should be written as:

You will be less likely to introduce bugs by having less code that does the same work. See if you can find other places in your code that can be equally reduced.
Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45
Thanks alot here.
This piece of code

Should be written as:

int serverCalc = tal2.calculate();
boolean answer = (serverCalc == kSvar);
out.writeBoolean(answer);
out.flush();



really helped me. It all started to work out much better. I have System.out.println on 15 different places around the code. It does help. But now I have another thing. When I get division by zero I have to deal with that in some way.
Where should I take care of it. Should I take care of it in the beginning were the numbers are created, i e in the class MathNumber.?

The constructor in MathNumber that creates the number looks like this:
Isn't that the best way to deal with it? I mean, somewhere here

Instead of waiting with it so that the client has to take care of it. I mean I don't want to send something that is not correct. It will only give me more problem. So it is better to take care of the problem at the source.??
But how do I do that? I have to in some way create a new mathnumber. Now it is created in MathClientHandler, line 40:



I tried to make a new method out of it; public void sendNumber() and then I changed the contructor in MathNumber to MathNumber(this)

But it didn't work out. Could you just maybe give me a hint about how to deal with it?

Anders



Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

Personally, I would handle it in both the server and the client. I would handle it in the client because it is the quickest way to repair the problem. I would handle it on the server because the server and client are to separate and independent applications, and what happens if client doesn't get it right, or if a different client (one I didn't write) comes along? Better safe than sorry.

The way I would fix it:

1) At the end of the MathNumber() method I would check if the operator is "/" and if it is, I would check number2 to see if it is 0. If it is, I would generate a new random number for number2.

2) In the MathClientHandlerNy class' run() method, after you read talSvar from the stream, you get number1, number2, and the operantor. At this point would check to see if the operator is "/" and if it is, if number2 is 0. If both true, then I would immediately respond with an error message written to the stream. You should have some protocol in place where your client reads the Stream and checks if the operation was successful, or if there was an error, and if there was an error what was it caused by. For example, instead of just writing the answer to the output stream, write a number: 0 means success, anything else means failure, and specific numbers mean specific types of failure. Then, if there was no failure, you write the boolean which indicates the correct answer or not. The client would first read the integer from the stream, and if it is 0, read the boolean to see if they got the answer correct. If the number is not 0, then they would look up what the response was and react accordingly (fix the numbers, fix the formats, etc...). But one of the key things is that your server should always return some signal to the client so the client knows to continue. So all those catch() clauses should have some response to the client telling them what problem occurred. (note, you don't need to have a code for every exception. Using -1 for 'general server error' could be fine, and you would use it, for example, with the NullPointerException. Also note that there are several exceptions that you can't respond this way with - SocketException, for example, and maybe IOException. I would try anyway, but the issue is moot because the same exception would probably occur on the client side).

A word on names: You have a method named MathNumber(). This is not very good practice. Method names should be actions (because methods do things) and should begin with a lowercase letter. So generateNumbers() is a better name. Using MathNumber is confusing because it is a noun and begins with a capital letter - both of which are used for class names (since classes represent things).
Anders Kviback
Ranch Hand

Joined: Mar 21, 2011
Posts: 45
Thanks a lot again.

I would say that the code almost works perfect now. Now division by zero is fixed as well. great!
But still it freezes occasionaly.

And what I can see from my system.out.println is that it is always at reading the stream, both server and client side.

MathClientHandler:


MathClient:




So, it stops at some of these points. Why?


MathNumber is not a method. It is a constructor. That's why it starts with a capital letter.


Thanks a lot again for helping me out here

// Anders
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

Anders Kviback wrote:... it freezes occasionaly.
...
So, it stops at some of these points. Why?

Unknown. You have to put system outs before every line and after every line to see exactly where it goes. and that includes catch() clauses. My guess is that you are still getting some exception in the server, not handling it well, and not sending a response to the client that an error occurred. So the client waits for a response and doesn't get it.


Anders Kviback wrote:MathNumber is not a method. It is a constructor. That's why it starts with a capital letter.

Ahh, my apologies, I should have realized it had no return type. It makes sense now


Another point on code clarity, though. You have this:

First, you should not compare a boolean against true or false, it is too easy to make a mistake (instead of if (b == true) use if (b), to avoid the occasional if (b = true) mistake, which is valid syntax but not what you want).
Second, you don't need to make the negative boolean comparison (else if (b == false) or else if (!b), because if it ever gets to the else clause then the if part must be true - so it is wasted code.
Finally, the lase clause (else without the if) is wasted code. How could the code ever reach there? Either b is true or b is false, there is no other option, a boolean only has two states. This even works with Booleans (capital B) as long as you make getting a null response impossible.

So I would write it as (remember, don't duplicate code!):
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 3950
    
  17

Steve Luke wrote:Unknown. You have to put system outs before every line and after every line to see exactly where it goes. and that includes catch() clauses. My guess is that you are still getting some exception in the server, not handling it well, and not sending a response to the client that an error occurred. So the client waits for a response and doesn't get it.


Well, maybe not every line, you could narrow it down sequentially to the point where you now exactly which branch through the method(s) where taken and which line was last started, but not ended. You can do this by putting fewer system outs to narrow down to some section of code, then add more system outs to trace the paths through that section, then do line-by-line system outs to get precise line causing the problem.

You could also do the same thing by using a debugger and scattering healthy doses of breakpoints throughout the code. Some IDEs will even let you add new breakpoints and remove others during execution, so you can put enough of them in to see what path through the code is being executed, then doing line-by-line through the problem section.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: gUI freezes - Deadlock problem?
 
Similar Threads
socket, gui, thread - problem
hand over number from gui to client class
Worker Threads
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
Closing a thread within a fixed thread pool