OutOfMemoryError with socket communication

[Andrew Thompson]

This causes the server to crash with an
OutOfMemoryError (tested on Linux with java 1.4.2 and 1.5.0). I haven't
succeeded in catching this error to prevent the crash.

Check this thread for tips on at least catching the
Error and doing some (tiny) thing about it..
<http://groups.google.com/group/comp...e_frm/thread/da2bab2d76fc1de/a7c395ea15ffdf8b>

 

[Thomas Hawtin]

sorry if this is too obvious, or has been discussed too many times. My
problem is with an ill-behaved client class crashing a server by closing
down its connection.

IIRC, we had the same problem a week or so ago, just in a different guise.

import java.net.*;
import java.io.*;

class TestServ extends Thread {

It a bad idea to extends Thread (or JFrame or similar for that matter).

ServerSocket ss;

It's generally a bad idea to use acronyms for variable names.
serverSocket would do just as well and be easier to make and sense of.

public static void main(String[] args) {
try {

Ick. Tabs? Just say no.

ServerSocket ss = new ServerSocket(6050);
TestServ ts = new TestServ(ss);
ts.start();

Well at least you didn't use thread.run();. OTOH, it wouldn't have made
any difference in this case.

} catch (IOException ex) {
ex.printStackTrace();
}
}

public TestServ(ServerSocket ss) {
this.ss = ss;
}

public void run() {
try {
while (true) {
Socket s = ss.accept();

The idea is that you create a new thread here (or hand it off to a pool
of threads). That way more than one client can connect.

s.setSoTimeout(100);
OutputStream out = s.getOutputStream();
InputStream in = s.getInputStream();
out.write("test".getBytes());
out.write('\0');
out.flush();
int c;
StringBuffer sb = new StringBuffer();
try {
while ((c = in.read()) != '\0') {

Here we are. What do the JavaDocs say no-args read returns?

sb.append((char)c);
}
} catch (SocketTimeoutException ex2) {
ex2.printStackTrace();
}
System.out.println(sb.toString());
}
} catch (Exception ex) {

It's usually a bad idea to catch Exception. If you want to catch
exceptions specified by method you've called, specify them again. If you
want to catch RuntimeExceptions write catch (RuntimeException. In the
rare case of you wanting to catch all exceptions, even those from
Class.newInstance, nasty JNI, bad generic casts and naughty byte code
manipulation, then catch Throwable.

ex.printStackTrace();
}

You are missing a finally to shutdown the socket if some random
exception occurs. Or even if it half closes.

}
}

The other thing to note is that some mickey mouse operating systems,
like Windows 95, didn't necessarily close sockets when a process exits.

Tom Hawtin

 

[Morten Alver]

Hi,

sorry if this is too obvious, or has been discussed too many times. My
problem is with an ill-behaved client class crashing a server by closing
down its connection.

I have two classes, a server and a client. When the server accepts the
connection, it sends a keyword, then expects to receive data from the
client. The client, on the other hand, just receives the keyword, then
closes down and quits. This causes the server to crash with an
OutOfMemoryError (tested on Linux with java 1.4.2 and 1.5.0). I haven't
succeeded in catching this error to prevent the crash. If the server times
out before the client quits, the server can recover and keep working.

I would like to hear some insight into why this happens, and how I can
prevent it. I want my server to be robust when dealing with ill-behaved
clients.


My classes:

------- TestServ.java -----------
import java.net.*;
import java.io.*;

class TestServ extends Thread {

ServerSocket ss;

public static void main(String[] args) {
try {
ServerSocket ss = new ServerSocket(6050);
TestServ ts = new TestServ(ss);
ts.start();
} catch (IOException ex) {
ex.printStackTrace();
}
}

public TestServ(ServerSocket ss) {
this.ss = ss;
}

public void run() {
try {
while (true) {
Socket s = ss.accept();
s.setSoTimeout(100);
OutputStream out = s.getOutputStream();
InputStream in = s.getInputStream();
out.write("test".getBytes());
out.write('\0');
out.flush();
int c;
StringBuffer sb = new StringBuffer();
try {
while ((c = in.read()) != '\0') {
sb.append((char)c);
}
} catch (SocketTimeoutException ex2) {
ex2.printStackTrace();
}
System.out.println(sb.toString());
}
} catch (Exception ex) {
ex.printStackTrace();
}
}
}


-------- Test.java ----------
import java.net.*;
import java.io.*;

class Test {

public static void main(String[] args) {
try {
Socket s = new Socket(InetAddress.getByName("localhost"), 6050);
InputStream in = s.getInputStream();
int c;
StringBuffer sb = new StringBuffer();
while ((c = in.read()) != '\0') {
sb.append((char)c);
}
in.close();
s.close();
System.out.println(sb.toString());
} catch (IOException ex) {
ex.printStackTrace();
}
}
}

 

[Morten Alver]

Check this thread for tips on at least catching the
Error and doing some (tiny) thing about it..
<http://groups.google.com/group/comp.lang.java.programmer/
browse_frm/thread/da2bab2d76fc1de/a7c395ea15ffdf8b>

Thanks for the tip! Actually, I was mistaken earlier - I find now that I
*can* catch the Error. It is thrown in the while loop of TestServ.java.
Calling printStackTrace() gives simply "java.lang.OutOfMemoryError: Java
heap space".

I might be missing something, but I find it quite unreasonable that the heap
space should run out for the server just because the client unexpectedly
closes its connection.

 

[Morten Alver]

Here we are. What do the JavaDocs say no-args read returns?

Damn it! I *knew* I must have forgotten something obvious! I am ashamed.

Thank you!

 

[Thomas Hawtin]

Damn it! I *knew* I must have forgotten something obvious! I am ashamed.

Everyone makes mistakes. Mistakes what you should expect to happen.
Often. Lots at the same time.

However, I think a quick go with a debugger or a few printfs would have
saved you.

Tom Hawtin

 

[Thomas Weidenfeller]

It a bad idea to extends Thread (or JFrame or similar for that matter).

It is an urban legend that extending JFrame (or other GUI classes) is a
bad idea. If you want a special JFrame-based reusable component it is a
good idea to subclass JFrame, and implement your special component in
accordance with the JavaBeans specification. That way you can add you
new component to the pallet of a bean-aware tool (IDE, GUI builder, etc.).

/Thomas

 

[Thomas Hawtin]

It is an urban legend that extending JFrame (or other GUI classes) is a
bad idea. If you want a special JFrame-based reusable component it is a
good idea to subclass JFrame, and implement your special component in
accordance with the JavaBeans specification. That way you can add you
new component to the pallet of a bean-aware tool (IDE, GUI builder, etc.).

Bollocks.

Dangerous bollocks at that. Extending classes without need is pure
incompetence.

Always prefer composition over inheritance. Prefer single inheritance of
interface.

JFrames tend to get inherited just for a piece of bad procedural coding.
If you inherit then you are forcing an unnatural organisation onto your
code. It stops the code being used for other components. You are
massively increasing dependencies. You can no longer use JApplets,
JInternalFrames, JDialogs or a JFrame that someone else has extended
from (perhaps even with a good reason). You can't move your code to use,
say, a panel of a tabbed pane. Flexibility is down the drain.
"Reusability" (and it seems like that word is the preserve of the
clueless) is down the drain. Don't specialise your code (but don't
generalise it either).

I've been reading "Killer Games Programming (in Java)". A lardy O'Reilly
book, big on code but light on solid information. It's very difficult to
see what is an API call and what is a call to the authors code. Do you
know all the methods in all the classes that extend Component? I've seen
isValid overridden. Seems eminently sensible. Isn't.

Thread is much the same (only much, much lighter). If you start
subclassing, then try introducing a thread pool...

Also, inheritance breaks encapsulation and is a general menace to society.

You've probably seen a lot of inheritance in a lot of books. I suggest
that those books are bad, and can safely be burnt.

You might like to check out:

Effective Java
-- Josh Bloch
Java Puzzlers: Traps, Pitfalls and Corner Cases
-- Neal Gafter & Josh Bloch

If you can get to see him live, Kevlin Henney is very good. Did a quick
google and found this (I wasn't looking for Java or Threads)

http://jupiter.robustserver.com/pipermail/cpp-threads_decadentplace.org.uk/2004-October/000059.html

Tom Hawtin

 

[Kenneth P. Turvey]

Always prefer composition over inheritance. Prefer single inheritance of
interface.

This is religion. There are uses for inheritance. That's why the standard
classes use it so much.

 

[Thomas Hawtin]

Thomas Hawtin wrote:




This is religion. There are uses for inheritance. That's why the standard
classes use it so much.

Sorry. In what way was I implying there weren't uses for inheritance??

However there are drawbacks. To use inheritance without reason is half
witted.

There are a few places where the standard classes use it unnecessarily.
java.util.Properties for instance. That's widely regarded as poor
design. As is extending Thread and the AWT 1.0 way of doing things.

Tom Hawtin

 

[jan V]

Bollocks.

Dangerous bollocks at that. Extending classes without need is pure
incompetence.

I agree, but instead of standing on your soap box on Hyde Corner for half an
hour, you might have taken the trouble of suggesting that people should
subclass JComponent or JPanel instead. [Yes, I know, I've done plenty of
time on Hyde Corner myself - mea culpa]

 

[Thomas Hawtin]

I agree, but instead of standing on your soap box on Hyde Corner for half an
hour, you might have taken the trouble of suggesting that people should
subclass JComponent or JPanel instead. [Yes, I know, I've done plenty of
time on Hyde Corner myself - mea culpa]

You misunderstand. Not only am I suggesting that Thread and JFrame not
be extended (if no method is overriden/implemented, the class is not a
decorator, there isn't an overriding issue with class size, etc) I am
suggesting that largely applies to any class.

(BTW: the arguments for extending JPanel instead of JComponent are
largely bogus.)

Tom Hawtin

 

[jan V]

(BTW: the arguments for extending JPanel instead of JComponent are

largely bogus.)

Bogus? What are you talking about?

Does Container.add(Component) look bogus to you? If you have to write an AWT
or Swing GUI, you'll have to subclass, I don't think you'll get very far
without subclassing classes that are several levels down from Object.

 

[Thomas Hawtin]

Bogus? What are you talking about?

Does Container.add(Component) look bogus to you? If you have to write an AWT
or Swing GUI, you'll have to subclass, I don't think you'll get very far
without subclassing classes that are several levels down from Object.

Sorry, what on Earth are you talking about?!

JComponent extends Container too!!

What I'm talking about is the oft repeated advice to extend JPanel
instead of JComponent (who would have guessed it?). From the
comp.lang.java.gui FAQ (http://www.physci.org/guifaq.jsp#4.1):

"Q4.1 What is the equivalent of AWT's Canvas in Swing?

"JPanel, if you want to have a "complete" component with a UI delegate
which handles opaque settings (if paintComponent() is correctly
overridden)."

Unfortunately JPanel does not necessarily have opaque set on it. Without
any further set up on my machine:

[tackline@localhost janv]$ cat Opaque.java
import javax.swing.JPanel;
import javax.swing.UIManager;

class Opaque {
public static void main(String[] args) throws Throwable {

UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
System.out.println(new JPanel().isOpaque());
}
}
[tackline@localhost janv]$ javac Opaque.java
[tackline@localhost janv]$ java Opaque
false

So, don't rely on what the PL&F for the particular JRE you are running
on your machine with your configuration does. Extend JPanel if you are
creating a panel and need to extend it (seems hideously unlikely to me).
If you genuinely are creating a new widget (say a tri-state 'toggle'
button), extend JComponent and go to all the bother of calling setOpaque
yourself.

Tom Hawtin