Reputation: 21755
This 3rd party script keeps causing heap memory exceptions:
byte[] forwardMessage(byte[] content) {
s = new Socket("172.17.0.30", 10001);
s.withStreams {InputStream input, OutputStream output ->
output.write content
return readRtsData(input)
}
}
byte[] readRtsData(input) {
def vplEndByte = 0xff
def inStream = new BufferedInputStream(input)
def bytes = []
while (bytes.isEmpty() || bytes.last() != vplEndByte) {
bytes.add(inStream.read())
}
bytes
}
That part of the script, which receives messages through TCP/IP after receiving the message, causes the following exception:
Exception in thread "Thread-2" org.codehaus.groovy.runtime.InvokerInvocationException: java.lang.OutOfMemoryError: Java heap space at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:92) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:234) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:272) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:880) at groovy.lang.Closure.call(Closure.java:279) at groovy.lang.Closure.call(Closure.java:292) at org.codehaus.groovy.runtime.DefaultGroovyMethods$6.run(DefaultGroovyMethods.java:11563) at java.lang.Thread.run(Thread.java:636) Caused by: java.lang.OutOfMemoryError: Java heap space at java.util.Arrays.copyOf(Arrays.java:2746) at java.util.ArrayList.ensureCapacity(ArrayList.java:187) at java.util.ArrayList.add(ArrayList.java:378) at sun.reflect.GeneratedMethodAccessor9.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:616) at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoCachedMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:229) at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:52) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125) at RTSGatewayServer.readRtsData(RTSGatewayServer.groovy:46) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:616) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:86) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:234) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:361) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:880) at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.callCurrent(PogoMetaClassSite.java:66) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:151) at RTSGatewayServer$_forwardMessage_closure2.doCall(RTSGatewayServer.groovy:35) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:616) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:86) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:234) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:272) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:880) at groovy.lang.Closure.call(Closure.java:279) at org.codehaus.groovy.runtime.DefaultGroovyMethods.withStreams(DefaultGroovyMethods.java:11462) at org.codehaus.groovy.runtime.dgm$658.invoke(Unknown Source)
I'm guessing there is a better more memory efficient way then using bytes.add(...)
?
If anybody can compare the result to what would happen in .NET that would be even better as I'm a .NET developer.
Upvotes: 1
Views: 3387
Reputation: 3168
If you read the contract,
InputStream.read():int
returns -1 when the stream ends.
Thus, if the stream doesn't feature the 0xff character (for example, if the stream ends before the 0xff is received), this code will just append the last byte received (e.g. -1) to that array forever!
Fix thusly:
while (bytes.isEmpty() || bytes.last() != vplEndByte || bytes.last != -1) {
//---other issues
While you should certainly put an upper bounds on this code to avoid OOMs, the specific issue you're having is with
def bytes = [] //[] is shorthand for "new ArrayList()"
This creates an ArrayList, an array backed data structure that starts by allocating an array and grows by doubling its size every time the previous capacity is reached. I believe the default size is only 10 elements.
Thus you're creating an awful lot of arrays here implicitly, which is what that mess of a stack trace is complaining about. In a high performance app with modest sized messages of a few K to an M, it's likely you're allocating new arrays faster than GC can keep up.
You should consider creating that arraylist with a sensible initial size to decrease the implicit object instantiation and introduce a sensible maximum bounds. You could also switch that collection to a linked list but you may experience a decrease in performance.
Upvotes: 1
Reputation: 353
Is it actually the data from the Socket that is causing OutofMemory?
If you are using Java 6, use jconsole to connect to the server and have a look at the heap.
Upvotes: 1
Reputation: 55907
So the script keeps reading and storing bytes until it sees 0xff.
This seems to be a flawed design. No matter how you tune the JVM you potentially eventually run out of memry. If the remote service chooses to send peta-bytes of deta before that 0xff then you're going to exhaust memory. My take is that you should always assume that the other participant may be broken or anti-social.
I would therefore set some upper bound on how much data I am prepared to accept. Then, if possible deal with the chunk you've receieved and go back and get the next chunk, or if it's not possible to process in chunks politely put out an error message and stop.
Bottom line: sanitise your inputs. Allowing an outside process to exhaust your memory is a bad thing. Don't believe them when they say "it can't happen".
Upvotes: 3